Expose client http scheme as an env variable

[aymanbagabas]

Add the ability to set the client HTTP scheme and default to the server's scheme when the environment variable is not provided.

Fixes: #37

[github-actions]

Download the artifacts for this pull request:

• Linux (arm64).zip
• Linux (x86_64).zip
• MacOS (arm64).zip
• MacOS (x86_64).zip
• Windows (x86_64).zip

Or use the following Docker image ghcr.io/charmbracelet/charm:devel-8276bf9

[rubiojr]

@aymanbagabas thanks for the fixes! one question I have, as I've been testing the branch.

I have to CHARM_HTTP_SCHEME=https ./charm fs ls / for the client to work, as I have the following environment variables exported for the server, so the server doesn't try to look for the certificates required to do the TLS termination when it boots:

CHARM_SERVER_HOST=my.charm.server
CHARM_SERVER_HTTP_SCHEME=http

Which means the fact that I have charm behind a proxy/tls terminator isn't fully transparent to the client.

Is that correct?

[aymanbagabas]

Yes, that's correct. Since you're using Caddy for tls termination, the charm server should run without tls (http). But you still need to instruct the charm client to use https to communicate properly with the server.

[rubiojr]

@aymanbagabas would it be useful/possible to have the client always use https by default unless instructed otherwise? or even something like the patch suggested in #37, so the client does the protocol upgrade automatically without user intervention? once you distribute the client you also need to make clients/users aware that they need to CHARM_HTTP_SCHEME=https, if you have the charm server behind a proxy.

Note: I think having the charm server designed to be the TLS termination is totally fine, I don't expect it to change because someone decided to complicate its life putting it behind a proxy 😉.

[rubiojr]

As an alternative to my hacky patch in #37, I've been testing adding a ServerURL to the client config struct so we don't need to specify host and scheme env vars separately, which works fine and reads better, but requires a larger changeset, since client's cfg.Host is used in a bunch of places. So something like:

diff --git a/client/client.go b/client/client.go
index 4fa77d6..8187e59 100644
--- a/client/client.go
+++ b/client/client.go
@@ -25,12 +25,10 @@ var nameValidator = regexp.MustCompile("^[a-zA-Z0-9]{1,50}$")
 
 // Config contains the Charm client configuration.
 type Config struct {
-       Host       string `env:"CHARM_HOST" default:"cloud.charm.sh"`
-       SSHPort    int    `env:"CHARM_SSH_PORT" default:"35353"`
-       HTTPPort   int    `env:"CHARM_HTTP_PORT" default:"35354"`
-       HTTPScheme string `env:"CHARM_HTTP_SCHEME" default:"https"`
-       Debug      bool   `env:"CHARM_DEBUG" default:"false"`
-       Logfile    string `env:"CHARM_LOGFILE" default:""`
+       ServerURL string `env:"CHARM_SERVER_URL" default:"https://cloud.charm.sh:35354"`
+       SSHPort   int    `env:"CHARM_SSH_PORT" default:"35353"`
+       Debug     bool   `env:"CHARM_DEBUG" default:"false"`
+       Logfile   string `env:"CHARM_LOGFILE" default:""`
 }

[rubiojr]

I realized today that the new JWKS URI will also be invalid when external TLS termination or proxying (docker) is happening, since the server scheme could be http (when not doing TLS termination) and the external/public charm HTTP port could be different than the one the charm server is binding to.

I'm solving it with a CHARM_SERVER_PUBLIC_URL for now, that could potentially be used by the client I guess.