Skip to content

Commit

Permalink
Retrieve, use and cache RootContainer for each DirectoryEntry object
Browse files Browse the repository at this point in the history
  • Loading branch information
Yvand committed Mar 8, 2019
1 parent dbb4017 commit a3f3e73
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 9 deletions.
11 changes: 11 additions & 0 deletions LDAPCP.Tests/AugmentationTests.cs
Expand Up @@ -21,6 +21,17 @@ public override void InitializeConfiguration()
ldapConn.GetGroupMembershipAsADDomainProp = true;
}
Config.Update();

//LDAPConnection coco = new LDAPConnection();
//coco.Path = "LDAP://contoso.local/DC=contoso,DC=local";
//coco.Username = @"contoso\spfarm";
//coco.Password = @"";
//coco.UserServerDirectoryEntry = false;
//coco.AugmentationEnabled = true;
//coco.GetGroupMembershipAsADDomainProp = true;
//Config.LDAPConnectionsProp.Clear();
//Config.LDAPConnectionsProp.Add(coco);
//Config.Update();
}
}

Expand Down
23 changes: 14 additions & 9 deletions LDAPCP/LDAPCP.cs
Expand Up @@ -945,24 +945,28 @@ protected virtual void SetLDAPConnection(Uri currentContext, LDAPConnection ldap
ldapConnection.Directory = Domain.GetComputerDomain().GetDirectoryEntry();
}

// Getting domain names information queries LDAP and may be slow, so they are retrieved once and cached
if (String.IsNullOrEmpty(ldapConnection.DomainName))
// Operations in this block do LDAP queries, so let's monitor their execution time
using (new SPMonitoredScope($"[{ProviderInternalName}] Get domain names / root container information about LDAP server \"{ldapConnection.Directory.Path}\"", 2000))
{
ClaimsProviderLogging.LogDebug($"Getting domain names information for LDAP connection {ldapConnection.Directory.Path}.");
// Retrieve FQDN and domain name of current DirectoryEntry
string domainName = String.Empty;
string domainFQDN = String.Empty;
try
{
// It may fail if for some reason LDAP server cannot be reached
// Operations below may fail if LDAP server cannot be reached
// Cache those values for the whole lifetime of the process, because getting them triggers LDAP queries in the background
OperationContext.GetDomainInformation(ldapConnection.Directory, out domainName, out domainFQDN);
ldapConnection.DomainName = domainName;
ldapConnection.DomainFQDN = domainFQDN;
if (ldapConnection.Directory.Properties.Contains("distinguishedName"))
{
ldapConnection.RootContainer = ldapConnection.Directory.Properties["distinguishedName"].Value.ToString();
}
}
catch (Exception ex)
{
ClaimsProviderLogging.LogException(ProviderInternalName, $"while getting domain names information for LDAP connection {ldapConnection.Directory.Path}", TraceCategory.Configuration, ex);
}
ldapConnection.DomainName = domainName;
ldapConnection.DomainFQDN = domainFQDN;
}
}

Expand Down Expand Up @@ -1180,7 +1184,6 @@ protected virtual List<SPClaim> GetGroupsFromActiveDirectory(LDAPConnection ldap
PrincipalContext principalContext = null;

// Build ContextOptions as documented in https://stackoverflow.com/questions/17451277/what-equivalent-of-authenticationtypes-secure-in-principalcontexts-contextoptio
// To use ContextOptions in constructor of PrincipalContext, "container" must also be set, but it can be null as per tests in https://stackoverflow.com/questions/2538064/active-directory-services-principalcontext-what-is-the-dn-of-a-container-o
ContextOptions contextOptions = new ContextOptions();
if ((ldapConnection.AuthenticationTypes & AuthenticationTypes.Sealing) == AuthenticationTypes.Sealing) contextOptions |= ContextOptions.Sealing;
if ((ldapConnection.AuthenticationTypes & AuthenticationTypes.Encryption) == AuthenticationTypes.Encryption) contextOptions |= ContextOptions.SecureSocketLayer;
Expand All @@ -1193,13 +1196,15 @@ protected virtual List<SPClaim> GetGroupsFromActiveDirectory(LDAPConnection ldap
using (new SPMonitoredScope($"[{ProviderInternalName}] Get AD Principal of user {currentContext.IncomingEntity.Value} from AD server \"{path}\"", 2000))
{
// Constructor of PrincipalContext does connect to LDAP server and may throw an exception if it fails, so it should be in try/catch
// To use ContextOptions in constructor of PrincipalContext, "container" must also be set, but it can be null as per tests in https://stackoverflow.com/questions/2538064/active-directory-services-principalcontext-what-is-the-dn-of-a-container-o
// Tests: if "container" is null, it always fails in PowerShell (tested only with AD) but somehow it works fine here
if (ldapConnection.UserServerDirectoryEntry)
{
principalContext = new PrincipalContext(ContextType.Domain, ldapConnection.DomainFQDN, null, contextOptions);
principalContext = new PrincipalContext(ContextType.Domain, ldapConnection.DomainFQDN, ldapConnection.RootContainer, contextOptions);
}
else
{
principalContext = new PrincipalContext(ContextType.Domain, ldapConnection.DomainFQDN, null, contextOptions, ldapConnection.Username, ldapConnection.Password);
principalContext = new PrincipalContext(ContextType.Domain, ldapConnection.DomainFQDN, ldapConnection.RootContainer, contextOptions, ldapConnection.Username, ldapConnection.Password);
}

// https://github.com/Yvand/LDAPCP/issues/22: UserPrincipal.FindByIdentity() doesn't support emails, so if IncomingEntity is an email, user needs to be retrieved in a different way
Expand Down
13 changes: 13 additions & 0 deletions LDAPCP/LDAPCPConfig.cs
Expand Up @@ -763,9 +763,21 @@ public bool GetGroupMembershipAsADDomainProp
public DirectoryEntry Directory;

public string Filter;
/// <summary>
/// The domain name, for example "contoso"
/// </summary>
public string DomainName;

/// <summary>
/// The fully qualified domain name, for example "contoso.local"
/// </summary>
public string DomainFQDN;

/// <summary>
/// The container on the store to use as the root of the context, for example "DC=contoso,DC=local"
/// </summary>
public string RootContainer;

public LDAPConnection()
{
}
Expand All @@ -788,6 +800,7 @@ internal LDAPConnection CopyPublicProperties()
Filter = this.Filter,
DomainName = this.DomainName,
DomainFQDN = this.DomainFQDN,
RootContainer = this.RootContainer,
};
}
}
Expand Down

0 comments on commit a3f3e73

Please sign in to comment.