From 778c16f3f1181b20ea474cda6732acacbb8644b5 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Wed, 27 Sep 2023 18:13:54 -0700 Subject: [PATCH 1/3] Account port number already included within server string --- .../Protocols/ldap/LdapConnection.Linux.cs | 7 ++-- .../tests/DirectoryServicesProtocolsTests.cs | 33 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/ldap/LdapConnection.Linux.cs b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/ldap/LdapConnection.Linux.cs index 0ce464ce2084d6..3993da251428a7 100644 --- a/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/ldap/LdapConnection.Linux.cs +++ b/src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/ldap/LdapConnection.Linux.cs @@ -60,8 +60,11 @@ private int InternalConnectToServer() } temp.Append(scheme); temp.Append(servers[i]); - temp.Append(':'); - temp.Append(directoryIdentifier.PortNumber); + if (!servers[i].Contains(':')) + { + temp.Append(':'); + temp.Append(directoryIdentifier.PortNumber); + } } if (temp.Length != 0) { diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs index b31cf294f4d217..9bc6232d8b6289 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs @@ -74,6 +74,39 @@ public void TestUnavailableNonCriticalExtension() _ = (SearchResponse) connection.SendRequest(searchRequest); // Does not throw } + + + private LdapConnection GetConnectionWithServerNameAndPort() + { + LdapDirectoryIdentifier directoryIdentifier = new LdapDirectoryIdentifier($"{LdapConfiguration.Configuration.ServerName}:{LdapConfiguration.Configuration.Port}", true, false); + NetworkCredential credential = new NetworkCredential(LdapConfiguration.Configuration.UserName, LdapConfiguration.Configuration.Password); + + LdapConnection connection = new LdapConnection(directoryIdentifier, credential) + { + AuthType = AuthType.Basic + }; + + // Set server protocol before bind; OpenLDAP servers default + // to LDAP v2, which we do not support, and will return LDAP_PROTOCOL_ERROR + connection.SessionOptions.ProtocolVersion = 3; + connection.SessionOptions.SecureSocketLayer = LdapConfiguration.Configuration.UseTls; + connection.Bind(); + + connection.Timeout = new TimeSpan(0, 3, 0); + return connection; + } + + [ConditionalFact(nameof(IsLdapConfigurationExist))] + public void TestServerWithPortNumber() + { + using LdapConnection connection = GetConnectionWithServerNameAndPort(); + + var searchRequest = new SearchRequest(LdapConfiguration.Configuration.SearchDn, "(objectClass=*)", SearchScope.Subtree); + + _ = (SearchResponse)connection.SendRequest(searchRequest); + // Shall succeed + } + [InlineData(60)] [InlineData(0)] [InlineData(-60)] From 8a144e27674f1d57d20fa8e8d8be9c1b21607f55 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Wed, 27 Sep 2023 18:25:28 -0700 Subject: [PATCH 2/3] Refactor the test --- .../tests/DirectoryServicesProtocolsTests.cs | 35 +++++++------------ 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs index 9bc6232d8b6289..c3ad42b88369d1 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs @@ -75,31 +75,10 @@ public void TestUnavailableNonCriticalExtension() // Does not throw } - - private LdapConnection GetConnectionWithServerNameAndPort() - { - LdapDirectoryIdentifier directoryIdentifier = new LdapDirectoryIdentifier($"{LdapConfiguration.Configuration.ServerName}:{LdapConfiguration.Configuration.Port}", true, false); - NetworkCredential credential = new NetworkCredential(LdapConfiguration.Configuration.UserName, LdapConfiguration.Configuration.Password); - - LdapConnection connection = new LdapConnection(directoryIdentifier, credential) - { - AuthType = AuthType.Basic - }; - - // Set server protocol before bind; OpenLDAP servers default - // to LDAP v2, which we do not support, and will return LDAP_PROTOCOL_ERROR - connection.SessionOptions.ProtocolVersion = 3; - connection.SessionOptions.SecureSocketLayer = LdapConfiguration.Configuration.UseTls; - connection.Bind(); - - connection.Timeout = new TimeSpan(0, 3, 0); - return connection; - } - [ConditionalFact(nameof(IsLdapConfigurationExist))] public void TestServerWithPortNumber() { - using LdapConnection connection = GetConnectionWithServerNameAndPort(); + using LdapConnection connection = GetConnection($"{LdapConfiguration.Configuration.ServerName}:{LdapConfiguration.Configuration.Port}"); var searchRequest = new SearchRequest(LdapConfiguration.Configuration.SearchDn, "(objectClass=*)", SearchScope.Subtree); @@ -807,6 +786,13 @@ private SearchResultEntry SearchUser(LdapConnection connection, string rootDn, s return null; } + private LdapConnection GetConnection(string server) + { + LdapDirectoryIdentifier directoryIdentifier = new LdapDirectoryIdentifier(server, true, false); + + return GetConnection(directoryIdentifier); + } + private LdapConnection GetConnection() { LdapDirectoryIdentifier directoryIdentifier = string.IsNullOrEmpty(LdapConfiguration.Configuration.Port) ? @@ -814,6 +800,11 @@ private LdapConnection GetConnection() new LdapDirectoryIdentifier(LdapConfiguration.Configuration.ServerName, int.Parse(LdapConfiguration.Configuration.Port, NumberStyles.None, CultureInfo.InvariantCulture), true, false); + return GetConnection(directoryIdentifier); + } + + private static LdapConnection GetConnection(LdapDirectoryIdentifier directoryIdentifier) + { NetworkCredential credential = new NetworkCredential(LdapConfiguration.Configuration.UserName, LdapConfiguration.Configuration.Password); LdapConnection connection = new LdapConnection(directoryIdentifier, credential) From 0a6b8102a7420f9f80d40775b5b0c7942f300e78 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 28 Sep 2023 09:18:06 -0700 Subject: [PATCH 3/3] Apply feedback --- .../tests/DirectoryServicesProtocolsTests.cs | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs b/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs index c3ad42b88369d1..00433bae9875c2 100644 --- a/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs +++ b/src/libraries/System.DirectoryServices.Protocols/tests/DirectoryServicesProtocolsTests.cs @@ -14,12 +14,12 @@ namespace System.DirectoryServices.Protocols.Tests { public partial class DirectoryServicesProtocolsTests { - internal static bool IsLdapConfigurationExist => LdapConfiguration.Configuration != null; - internal static bool IsActiveDirectoryServer => IsLdapConfigurationExist && LdapConfiguration.Configuration.IsActiveDirectoryServer; + internal static bool LdapConfigurationExists => LdapConfiguration.Configuration != null; + internal static bool IsActiveDirectoryServer => LdapConfigurationExists && LdapConfiguration.Configuration.IsActiveDirectoryServer; - internal static bool IsServerSideSortSupported => IsLdapConfigurationExist && LdapConfiguration.Configuration.SupportsServerSideSort; + internal static bool IsServerSideSortSupported => LdapConfigurationExists && LdapConfiguration.Configuration.SupportsServerSideSort; - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestInvalidFilter() { using LdapConnection connection = GetConnection(); @@ -33,7 +33,7 @@ public void TestInvalidFilter() Assert.Equal(/* LdapError.FilterError */ 0x57, ex.ErrorCode); } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestInvalidSearchDn() { using LdapConnection connection = GetConnection(); @@ -47,7 +47,7 @@ public void TestInvalidSearchDn() Assert.Equal(ResultCode.InvalidDNSyntax, ex.Response.ResultCode); } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestUnavailableCriticalExtension() { using LdapConnection connection = GetConnection(); @@ -63,7 +63,7 @@ public void TestUnavailableCriticalExtension() Assert.Equal(ResultCode.UnavailableCriticalExtension, ex.Response.ResultCode); } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestUnavailableNonCriticalExtension() { using LdapConnection connection = GetConnection(); @@ -75,7 +75,7 @@ public void TestUnavailableNonCriticalExtension() // Does not throw } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestServerWithPortNumber() { using LdapConnection connection = GetConnection($"{LdapConfiguration.Configuration.ServerName}:{LdapConfiguration.Configuration.Port}"); @@ -89,7 +89,7 @@ public void TestServerWithPortNumber() [InlineData(60)] [InlineData(0)] [InlineData(-60)] - [ConditionalTheory(nameof(IsLdapConfigurationExist))] + [ConditionalTheory(nameof(LdapConfigurationExists))] public void TestSearchWithTimeLimit(int timeLimit) { using LdapConnection connection = GetConnection(); @@ -107,7 +107,7 @@ public void TestSearchWithTimeLimit(int timeLimit) } } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestAddingOU() { using (LdapConnection connection = GetConnection()) @@ -129,7 +129,7 @@ public void TestAddingOU() } } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestDeleteOU() { using (LdapConnection connection = GetConnection()) @@ -154,7 +154,7 @@ public void TestDeleteOU() } } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestAddAndModifyAttribute() { using (LdapConnection connection = GetConnection()) @@ -189,7 +189,7 @@ public void TestAddAndModifyAttribute() } } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestNestedOUs() { using (LdapConnection connection = GetConnection()) @@ -220,7 +220,7 @@ public void TestNestedOUs() } } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestAddUser() { using (LdapConnection connection = GetConnection()) @@ -272,7 +272,7 @@ public void TestAddUser() } } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestAddingMultipleAttributes() { using (LdapConnection connection = GetConnection()) @@ -355,7 +355,7 @@ public void TestAddingMultipleAttributes() } } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestMoveAndRenameUser() { using (LdapConnection connection = GetConnection()) @@ -414,7 +414,7 @@ public void TestMoveAndRenameUser() } } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestAsyncSearch() { using (LdapConnection connection = GetConnection()) @@ -525,7 +525,7 @@ public static IEnumerable TestCompareRequestTheory_TestData() yield return new object[] { "http://example.com/", "http://false/"u8.ToArray(), ResultCode.CompareFalse }; } - [ConditionalTheory(nameof(IsLdapConfigurationExist))] + [ConditionalTheory(nameof(LdapConfigurationExists))] [MemberData(nameof(TestCompareRequestTheory_TestData))] public void TestCompareRequestTheory(object value, object assertion, ResultCode compareResult) { @@ -558,7 +558,7 @@ public void TestCompareRequestTheory(object value, object assertion, ResultCode } } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestCompareRequest() { using (LdapConnection connection = GetConnection()) @@ -682,7 +682,7 @@ public void TestSortedSearch() } } - [ConditionalFact(nameof(IsLdapConfigurationExist))] + [ConditionalFact(nameof(LdapConfigurationExists))] public void TestMultipleServerBind() { LdapDirectoryIdentifier directoryIdentifier = string.IsNullOrEmpty(LdapConfiguration.Configuration.Port) ? @@ -788,7 +788,7 @@ private SearchResultEntry SearchUser(LdapConnection connection, string rootDn, s private LdapConnection GetConnection(string server) { - LdapDirectoryIdentifier directoryIdentifier = new LdapDirectoryIdentifier(server, true, false); + LdapDirectoryIdentifier directoryIdentifier = new LdapDirectoryIdentifier(server, fullyQualifiedDnsHostName: true, connectionless: false); return GetConnection(directoryIdentifier); } @@ -796,10 +796,10 @@ private LdapConnection GetConnection(string server) private LdapConnection GetConnection() { LdapDirectoryIdentifier directoryIdentifier = string.IsNullOrEmpty(LdapConfiguration.Configuration.Port) ? - new LdapDirectoryIdentifier(LdapConfiguration.Configuration.ServerName, true, false) : + new LdapDirectoryIdentifier(LdapConfiguration.Configuration.ServerName, fullyQualifiedDnsHostName: true, connectionless: false) : new LdapDirectoryIdentifier(LdapConfiguration.Configuration.ServerName, int.Parse(LdapConfiguration.Configuration.Port, NumberStyles.None, CultureInfo.InvariantCulture), - true, false); + fullyQualifiedDnsHostName: true, connectionless: false); return GetConnection(directoryIdentifier); }