From 4ce140b481012508f564262dad3e8b4ac0dc2c17 Mon Sep 17 00:00:00 2001 From: Yvan Duhamel Date: Tue, 12 Mar 2019 10:58:45 +0100 Subject: [PATCH] Do more fine-grained test when excluding a LDAP user missing the identity attribute --- CHANGELOG.md | 6 +++++- LDAPCP/LDAPCP.cs | 32 ++++++++++++++++---------------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90fa2ad..9153360 100644 --- 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 index b6ca486..d20e204 100644 --- a/LDAPCP/LDAPCP.cs +++ b/LDAPCP/LDAPCP.cs @@ -708,33 +708,33 @@ protected virtual ConsolidatedResultCollection ProcessLdapResults(OperationConte IEnumerable LDAPResultPropertyNames = LDAPResultProperties.PropertyNames.Cast(); // 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().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().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().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