chore: support mtls settings #2443
Conversation
rajcspsg
requested review from
amitksingh1490,
jdegoes,
vigoo and
adamgfraser
as code owners
rajcspsg
force-pushed
the
issue-2211
branch
from
b8c1019 to
258787c
Compare
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2443 +/- ##
==========================================
- Coverage 64.30% 64.27% -0.04%
==========================================
Files 139 139
Lines 8198 8207 +9
Branches 1530 1497 -33
==========================================
+ Hits 5272 5275 +3
- Misses 2926 2932 +6 ☔ View full report in Codecov by Sentry. |
rajcspsg
force-pushed
the
issue-2211
branch
from
258787c to
5b85fea
Compare
|
@adamgfraser still I need to write test. could you have quick glance whether the approach is correct. |
|
@danestig Can you review and ensure this meets your needs? |
|
Hello! Thanks for taking a stab at this @rajcspsg. I have 2 concerns:
|
|
@danestig Sorry I missed your comment. I did mistake I updating the ClientSSLConfig config instead of ServerSSLConfig. |
|
@rajcspsg Yes, I believe that would solve my issue. Thank you. |
| final case class FromTrustStoreResource(trustStorePath: String, trustStorePassword: String) extends ClientSSLConfig | ||
| final case class FromTrustStoreFile(trustStorePath: String, trustStorePassword: String) extends ClientSSLConfig | ||
| sealed trait ClientAuth | ||
| case object RequireClientAuth extends ClientAuth |
There was a problem hiding this comment.
@rajcspsg Can you put these in the companion object of ClientAuth?
There was a problem hiding this comment.
Also, once you put them there, they can have shorter names, e.g.:
object ClientAuth {
case object Required extends ClientAuth
case object None extends ClientAuth
case object Optional extends ClientAuth
}There was a problem hiding this comment.
@jdegoes sure. I've to set this config on ServerSslContext not on ClientSSLContext
There was a problem hiding this comment.
I'll update my PR shortly
|
@jdegoes @danestig @adamgfraser I updated the PR to add clientAuth config on SSLServerContext. |
rajcspsg
force-pushed
the
issue-2211
branch
2 times, most recently
from
043cd94 to
c2fbba7
Compare
| new SSLConfig(HttpBehaviour.Redirect, data, Provider.JDK, None) | ||
|
|
||
| def apply(data: Data, clientAuth: ClientAuth): SSLConfig = | ||
| new SSLConfig(HttpBehaviour.Redirect, data, Provider.JDK, Some(clientAuth)) | ||
|
|
There was a problem hiding this comment.
@jdegoes @adamgfraser I tried to combine both the apply (and other duplicate functions below)
into single function like
def apply(data: Data, clientAuth: Option[ClientAuth] = None): SSLConfig =
new SSLConfig(HttpBehaviour.Redirect, data, Provider.JDK, clientAuth)But I'm getting below error -
/home/raj/Coding/scala/zio-http/zio-http/src/main/scala/zio/http/SSLConfig.scala:39:8: in object SSLConfig, multiple overloaded alternatives of method fromFile define default arguments.
I did workaround to make it compile. Not sure why the first option didn't work
closes #2211