SslServerAuthenticationOptions - update documentation #4367
Conversation
| </ReturnValue> | ||
| <Docs> | ||
| <summary>To be added.</summary> | ||
| <summary>Gets or sets a <see cref="T:System.Boolean" /> value that specifies whether the client is asked for a certificate for authentication. Note that this is only a request -- if no certificate is provided, the server still accepts the connection request.</summary> |
There was a problem hiding this comment.
I think you can skip the <see cref="T:System.Boolean" /> here.
a certificate for authentication --> an authentication certificate maybe???
if no certificate is provided, the server still accepts the connection request. really? I'm looking into some of ours code using this property and it doesn't seem to me like that. But I might be reading the code wrong, or looking at the wrong place, so feel free to disregard this.
There was a problem hiding this comment.
I borrowed it from there: https://review.docs.microsoft.com/en-us/dotnet/api/system.net.security.sslstream.authenticateasserverasync?view=netcore-1.1&branch=pr-en-us-4350#System_Net_Security_SslStream_AuthenticateAsServerAsync_System_Security_Cryptography_X509Certificates_X509Certificate_System_Boolean_System_Security_Authentication_SslProtocols_System_Boolean_ . The enabledSslProtocols parameter has the same meaning as SslServerAuthenticationOptions.ClientCertificateRequired in overloaded AuthenticateAsServerXXX
| </ReturnValue> | ||
| <Docs> | ||
| <summary>To be added.</summary> | ||
| <summary>Gets or sets a <see cref="T:System.Net.Security.RemoteCertificateValidationCallback" /> delegate responsible for validating the certificate supplied by the remote party.</summary> |
There was a problem hiding this comment.
The callback is used as certificate validation override, i.e. the user can provide their own logic to validate the certificate instead of using the default behavior, see: https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs#L1007.
It will still work if nothing is provided.
|
I tried to look into usages of this class to add deeper documentation for the properties. Though, it might not be something desired here since it's API reference docs and also the class might potentially be used in different scenarios/contexts than |
* SslApplicationProtocol: update documention * Update xml/System.Net.Security/SslApplicationProtocol.xml Co-authored-by: Maira Wenzel <mairaw@microsoft.com> * Update xml/System.Net.Security/SslApplicationProtocol.xml Co-authored-by: Maira Wenzel <mairaw@microsoft.com> * Fix see reference * Add @ManickaP comments raised in #4367 * Remove space from remarks and Co-authored-by: Jan Jahoda <jajahoda@.microsoft.com> Co-authored-by: Maira Wenzel <mairaw@microsoft.com>
There was a problem hiding this comment.
There's a typo in the type's summary: a values. Indefinite article cannot be used with plural nouns (https://en.wiktionary.org/wiki/indefinite_article: in the plural, an indefinite article isn't used at all.)
I think this should read: a value, singular.
I know it's not part of your changes, but could this be fixed either here or in a separate PR.
3 participants
Summary
Collect all changes from #3916 related to SslServerAuthenticationOptions