ClientCertificateMode now read from config #18759
Conversation
kuns200
requested review from
analogrelay,
halter73,
jkotalik and
Tratcher
as code owners
ghost
added
the
|
|
||
| private IConfiguration _configuration; | ||
| private IDictionary<string, CertificateConfig> _certificates; | ||
| private IList<EndpointConfig> _endpoints; | ||
| private EndpointDefaults _endpointDefaults; | ||
|
|
||
| private string _clientCertificateMode; |
There was a problem hiding this comment.
Can we make this type ClientCertificateMode?
| } | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Nit: remove the extra newline above and below property.
| @@ -65,6 +80,10 @@ public IEnumerable<EndpointConfig> Endpoints | |||
| } | |||
| } | |||
|
|
|||
| private void ReadClientCertificateMode() | |||
There was a problem hiding this comment.
This should do the enum parsing instead of LoadClientCertificateMode in KestrelConfigurationLoader.
| private void ReadClientCertificateMode() | ||
| { | ||
| _clientCertificateMode = _configuration[ClientCertificateModeKey]; | ||
| } |
|
|
||
| private static HttpProtocols? ParseProtocols(string protocols) | ||
| private static HttpProtocols? ParseProtocols(string protocols) |
| @@ -12,6 +12,28 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Tests | |||
| { | |||
| public class ConfigurationReaderTests | |||
| { | |||
|
|
|||
| var reader = new ConfigurationReader(config); | ||
| var clientCertificateMode = reader.ClientCertificateMode; | ||
| Assert.NotNull(clientCertificateMode); | ||
| } |
| var reader = new ConfigurationReader(config); | ||
| var clientCertificateMode = reader.ClientCertificateMode; | ||
| Assert.Null(clientCertificateMode); | ||
| } |
|
ok . will make changes. |
|
@kuns200 Do you have time to look at this again? Thanks. |
|
@kuns200 Are you still interested in doing this? |
|
@kuns200 If you're still interested you can open a new PR |
|
I was busy last year with few things at home. Can I look this now? Or is it done? |
|
Hi @kuns200. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
|
Looks like it got done via #24076 |
amcasey
added
area-networking
Client certificate mode can now be read from config file.
It uses,
connectionreader.csAddresses #18660