-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add support for LDAPTLS_CACERTDIR \ TrustedCertificateDirectory #111877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
510c98b
4a4427e
51070ed
5c64144
1940a2b
7a11a43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -382,6 +382,12 @@ public partial class LdapSessionOptions | |||||||||
| internal LdapSessionOptions() { } | ||||||||||
| public bool AutoReconnect { get { throw null; } set { } } | ||||||||||
| public string DomainName { get { throw null; } set { } } | ||||||||||
| [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")] | ||||||||||
| [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] | ||||||||||
| [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] | ||||||||||
| [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] | ||||||||||
|
||||||||||
| [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")] | |
| [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] | |
| [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] | |
| [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Buyaa! Removed those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed from source but not from ref
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again - done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still seeing these, @steveharter. I think we can omit the attributes already defined at the assembly level, and just add the one additional windows one here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, for some reason was looking at subset of commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming the previously excluded browser;android;ios;tvos were because the browser and mobile platforms don't support LDAP \ DirectoryServices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we expose TrustedCertificatesDirectory in the netstandard2.0 assembly. Then on .NETFramework we expect that to be replaced with the inbox assembly from netfx which doesn't have this API.
We may want to ifdef this #if NET to only expose the API on .NET versions where it is implemented.
API didn't catch this because we don't compare netstandard2.0 to netframework if a package doesn't have an assembly for it. cc @ViktorHofer maybe we should add a comparison tuple when we see the empty folder?
ericstj marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it changed but, we were talking to make VerifyServerCertificate windows only with this change
| public System.DirectoryServices.Protocols.VerifyServerCertificateCallback VerifyServerCertificate { get { throw null; } set { } } | |
| [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] | |
| public System.DirectoryServices.Protocols.VerifyServerCertificateCallback VerifyServerCertificate { get { throw null; } set { } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Buyaa! I'll put up another PR for that (per this PR's description I elected not to do any of that, but VerifyServerCertificateCallback has caused a lot of pain so I will do that one-off).
Uh oh!
There was an error while loading. Please reload this page.