Change LoadDomainInfo to honor requested LDAP port #89787
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dotnet#65894 LoadDomainInfo builds an LDAP Uri that ignores the requested port. If LDAPS/636 is requested, traffic still goes to 389 for this request.
ghost
added
area-System.DirectoryServices
community-contribution
|
Tagging subscribers to this area: @dotnet/area-system-directoryservices, @jay98014 Issue DetailsLoadDomainInfo builds an LDAP Uri that ignores the requested port. If LDAPS/636 is requested, traffic still goes to 389 for this request. For environments where port 389 is blocked, this causes calls such as GroupPrincipal.GetMembers() to fail. This PR will use the specified port for the call, 389 if no port is specified, or 636 if LDAPS is specified.
|
Fixing CA1311
|
PTAL @jay98014 and @kumarravi |
ericstj
requested review from
kumarravik78c
and removed request for
kumarravi
kumarravik78c
approved these changes
Sep 12, 2023
steveharter
approved these changes
Sep 13, 2023
rhuijben
reviewed
Sep 13, 2023
|
|
||
| // Pull the requested port number | ||
| Uri ldapUri = new Uri(this.ctxBase.Path); | ||
| int port = ldapUri.Port != -1 ? ldapUri.Port : (ldapUri.Scheme.ToUpperInvariant() == "LDAPS" ? 636 : 389); |
There was a problem hiding this comment.
String.Equals(..., StringComparison.OrdinalIgnoreCase) avoids an unnecessary allocation.
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #65894
LoadDomainInfo builds an LDAP Uri that ignores the requested port. If LDAPS/636 is requested, traffic still goes to 389 for this request. For environments where port 389 is blocked, this causes calls such as GroupPrincipal.GetMembers() to fail when the group is Domain Local or Universal. We do not seem to call LoadDomainInfo for Global groups.
This PR will use the specified port for the call, 389 if no port is specified, or 636 if LDAPS is specified.