Do more fine-grained test when excluding a LDAP user missing the iden…

…tity attribute
Yvand · Mar 12, 2019 · 4ce140b · 4ce140b
1 parent 064042e
commit 4ce140b
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,7 +14,11 @@
 * Cache result returned by FileVersionInfo.GetVersionInfo() to avoid potential hangs
 * Add property AzureCPConfig.MaxSearchResultsCount to set max number of results returned to SharePoint during a search
 * Cache domain name and domain FQDN of each LDAP Connection, to avoid repetitive and potentially slow queries to LDAP servers
-* Deprecate method LDAPCP.SetLDAPConnection called each time a LDAP operation is about to occur. Instead, added a separate method LDAPCP.SetLDAPConnection, called only during initialization of the configuration. Domain name and domain FQDN are retrieved at this time.
+* Deprecate method LDAPCP.SetLDAPConnection called each time a LDAP operation is about to occur. Instead, added a separate method LDAPCP.SetLDAPConnection, called only during initialization of the configuration. Domain name, domain FQDN and distinguishedName are retrieved and cached here.
+* Improve global performance by caching domain name and domain FQDN of each LDAP Connection, to avoid repetitive and potentially slow queries to LDAP servers
+* Update logging during augmentation, and split various LDAP operations into different SPMonitoredScope
+* Update augmentatikon by getting, using and caching RootContainer for each DirectoryEntry object
+* Do more fine-grained test when excluding a LDAP user missing the identity attribute
 * Update NuGet package NUnit to v3.11
 * Update NuGet package NUnit3TestAdapter to v3.13
 * Update NuGet package CsvTools to v1.0.12

diff --git a/LDAPCP/LDAPCP.cs b/LDAPCP/LDAPCP.cs
@@ -708,33 +708,33 @@ protected virtual ConsolidatedResultCollection ProcessLdapResults(OperationConte
                 IEnumerable<string> LDAPResultPropertyNames = LDAPResultProperties.PropertyNames.Cast<string>();
 
                 // Issue https://github.com/Yvand/LDAPCP/issues/16: If current result is a user, ensure LDAP attribute of identity ClaimTypeConfig exists in current LDAP result
+                bool isUserWithNoIdentityAttribute = false;
                 if (LDAPResultProperties[LDAPObjectClassName].Cast<string>().Contains(IdentityClaimTypeConfig.LDAPClass, StringComparer.InvariantCultureIgnoreCase))
                 {
                     // This is a user: check if his identity LDAP attribute (e.g. mail or sAMAccountName) is present
                     if (!LDAPResultPropertyNames.Contains(IdentityClaimTypeConfig.LDAPAttribute, StringComparer.InvariantCultureIgnoreCase))
                     {
-                        ClaimsProviderLogging.Log($"[{ProviderInternalName}] Ignoring a user because he doesn't have the LDAP attribute '{IdentityClaimTypeConfig.LDAPAttribute}'", TraceSeverity.VerboseEx, EventSeverity.Information, TraceCategory.LDAP_Lookup);
-                        continue;
+                        // This may match a result like PrimaryGroupID, which has EntityType "Group", but LDAPClass "User"
+                        // So it cannot be ruled out immediately, but needs be tested against each ClaimTypeConfig
+                        //ClaimsProviderLogging.Log($"[{ProviderInternalName}] Ignoring a user because he doesn't have the LDAP attribute '{IdentityClaimTypeConfig.LDAPAttribute}'", TraceSeverity.VerboseEx, EventSeverity.Information, TraceCategory.LDAP_Lookup);
+                        //continue;
+                        isUserWithNoIdentityAttribute = true;
                     }
                 }
-                else
-                {
-                    // This is a group: check if the LDAP attribute used to create groups entities is present
-                    // EDIT: since groups can have multiple claim types, this check does not make sense
-                    //if (MainGroupClaimTypeConfig != null && !LDAPResultPropertyNames.Contains(MainGroupClaimTypeConfig.LDAPAttribute, StringComparer.InvariantCultureIgnoreCase))
-                    //{
-                    //    ClaimsProviderLogging.Log($"[{ProviderInternalName}] Ignoring a group because it doesn't have the LDAP attribute '{MainGroupClaimTypeConfig.LDAPAttribute}'", TraceSeverity.VerboseEx, EventSeverity.Information, TraceCategory.LDAP_Lookup);
-                    //    continue;
-                    //}
-                }
 
                 foreach (ClaimTypeConfig ctConfig in ctConfigs)
                 {
-                    // Check if LDAPClass of current ClaimTypeConfig matches the current LDAP result
-                    if (!LDAPResultProperties[LDAPObjectClassName].Cast<string>().Contains(ctConfig.LDAPClass, StringComparer.InvariantCultureIgnoreCase)) continue;
+                    // Skip if: current config is for users AND LDAP result is a user AND LDAP result doesn't have identity attribute set
+                    if (ctConfig.EntityType == DirectoryObjectType.User && isUserWithNoIdentityAttribute)
+                        continue;
+
+                    // Skip if: LDAPClass of current config does not match objectclass of LDAP result
+                    if (!LDAPResultProperties[LDAPObjectClassName].Cast<string>().Contains(ctConfig.LDAPClass, StringComparer.InvariantCultureIgnoreCase))
+                        continue;
 
-                    // Check if current LDAP result contains LDAP attribute of current attribute
-                    if (!LDAPResultPropertyNames.Contains(ctConfig.LDAPAttribute, StringComparer.InvariantCultureIgnoreCase)) continue;
+                    // Skip if: LDAPAttribute of current config is not found in LDAP result
+                    if (!LDAPResultPropertyNames.Contains(ctConfig.LDAPAttribute, StringComparer.InvariantCultureIgnoreCase))
+                        continue;
 
                     // Get value with of current LDAP attribute
                     // TODO: investigate https://github.com/Yvand/LDAPCP/issues/43