OAuth now uses client TLS certs (if configured) #5042
Conversation
There was a problem hiding this comment.
Your contribution is appreciated!
Is it possible for us to use the existing ClientCAFile property instead of adding these new ones? RIght now, it's consumed when TLS is enabled, but it seems like we should make it always apply instead and add the oauth2.HTTPClient to the context.
If you need help or have questions, feel free to ping me! Happy to add this (and we can do a release afterward so you can use it).
| @@ -1248,3 +1253,21 @@ func startBuiltinPostgres(ctx context.Context, cfg config.Root, logger slog.Logg | |||
| } | |||
| return connectionURL, ep.Stop, nil | |||
| } | |||
|
|
|||
| func handleOauth2ClientCertificates(cfg *codersdk.DeploymentConfig, ctx context.Context) (context.Context, error) { | |||
| if cfg.TLS.ClientCertFile.Value != "" && cfg.TLS.ClientKeyFile.Value != "" { | |||
There was a problem hiding this comment.
We already have ClientCAFile as a configuration option, so you should be able to remove the configuration changes here!
There was a problem hiding this comment.
I'm going to expand on my use-case a bit more because I think it's slightly different
So with my changes in the config file we have:
CertFilesKeyFilesClientCAFileClientCertFile(new)ClientKeyFile(new)
In my org, I have been given a wildcard cert *.coder.normana10.example.com and a "system/identity" cert normana10.example.com
I need to use the wildcard cert for all "serving" and the "identity" cert for all external calls that Coder makes
(I'm not going to say this is "correct", but it's just how my org does things)
So if I wanted Coder to terminate TLS my config would look like:
CertFiles=/path/to/wildcard.certKeyFiles=/path/to/wildcard.keyClientCAFile=/path/to/ca.cert(Only used to verify client certs ifClientAuthis set to verify)ClientCertFile(new) =/path/to/identity.certClientKeyFile(new) =/path/to/identity.key
So ClientCAFile (as far as I've seen) looks like it's just used to verify clients connecting to Coder. Where I'm looking to set the client certs Coder uses when connecting to external services
If that makes sense?
For full transparency, my actual config doesn't have Coder terminating TLS, so realistically I'd just have my two new configs set
There was a problem hiding this comment.
All makes sense! I appreciate the context... the changes look good to me!
| @@ -1248,3 +1253,21 @@ func startBuiltinPostgres(ctx context.Context, cfg config.Root, logger slog.Logg | |||
| } | |||
| return connectionURL, ep.Stop, nil | |||
| } | |||
|
|
|||
| func handleOauth2ClientCertificates(ctx context.Context, cfg *codersdk.DeploymentConfig) (context.Context, error) { | |||
| if cfg.TLS.ClientCertFile.Value != "" && cfg.TLS.ClientKeyFile.Value != "" { | |||
There was a problem hiding this comment.
nit (not blocking merge): You could invert the control-flow here to reduce indentation!
Hi,
I'm trying to get Coder deployed within my organization but our OpenID Connect provider requires valid TLS client certificates before it will respond to the
.well-knownHTTP call that theoidclibrary makesThese changes configure the
oidclibrary to use (optional and configurable) client TLS certificatesI've verified these changes work with my personal/homelab OpenID Connect provider when I configure my reverse proxy to require TLS client certs
(Let me know if I missed anything, I'm new to Go 😄)
Also let me know if there's some glaringly obvious Go-ish way to do this with like environment variables or command line args that I'm unaware of
Thanks!