diff --git a/cli/clibase/cmd.go b/cli/clibase/cmd.go index d678910887..ba359faae6 100644 --- a/cli/clibase/cmd.go +++ b/cli/clibase/cmd.go @@ -226,6 +226,15 @@ func (inv *Invocation) SignalNotifyContext(parent context.Context, signals ...os return inv.signalNotifyContext(parent, signals...) } +func (inv *Invocation) WithTestParsedFlags( + _ testing.TB, // ensure we only call this from tests + parsedFlags *pflag.FlagSet, +) *Invocation { + return inv.with(func(i *Invocation) { + i.parsedFlags = parsedFlags + }) +} + func (inv *Invocation) Context() context.Context { if inv.ctx == nil { return context.Background() diff --git a/cli/server.go b/cli/server.go index ab2f7bb01e..896b7b78ef 100644 --- a/cli/server.go +++ b/cli/server.go @@ -2319,12 +2319,7 @@ func ConfigureHTTPServers(logger slog.Logger, inv *clibase.Invocation, cfg *code return nil, xerrors.New("tls address must be set if tls is enabled") } - // DEPRECATED: This redirect used to default to true. - // It made more sense to have the redirect be opt-in. - if inv.Environ.Get("CODER_TLS_REDIRECT_HTTP") == "true" || inv.ParsedFlags().Changed("tls-redirect-http-to-https") { - logger.Warn(ctx, "--tls-redirect-http-to-https is deprecated, please use --redirect-to-access-url instead") - cfg.RedirectToAccessURL = cfg.TLS.RedirectHTTP - } + redirectHTTPToHTTPSDeprecation(ctx, logger, inv, cfg) tlsConfig, err := configureServerTLS( ctx, @@ -2374,6 +2369,31 @@ func ConfigureHTTPServers(logger slog.Logger, inv *clibase.Invocation, cfg *code return httpServers, nil } +// redirectHTTPToHTTPSDeprecation handles deprecation of the --tls-redirect-http-to-https flag and +// "related" environment variables. +// +// --tls-redirect-http-to-https used to default to true. +// It made more sense to have the redirect be opt-in. +// +// Also, for a while we have been accepting the environment variable (but not the +// corresponding flag!) "CODER_TLS_REDIRECT_HTTP", and it appeared in a configuration +// example, so we keep accepting it to not break backward compat. +func redirectHTTPToHTTPSDeprecation(ctx context.Context, logger slog.Logger, inv *clibase.Invocation, cfg *codersdk.DeploymentValues) { + truthy := func(s string) bool { + b, err := strconv.ParseBool(s) + if err != nil { + return false + } + return b + } + if truthy(inv.Environ.Get("CODER_TLS_REDIRECT_HTTP")) || + truthy(inv.Environ.Get("CODER_TLS_REDIRECT_HTTP_TO_HTTPS")) || + inv.ParsedFlags().Changed("tls-redirect-http-to-https") { + logger.Warn(ctx, "⚠️ --tls-redirect-http-to-https is deprecated, please use --redirect-to-access-url instead") + cfg.RedirectToAccessURL = cfg.TLS.RedirectHTTP + } +} + // ReadExternalAuthProvidersFromEnv is provided for compatibility purposes with // the viper CLI. func ReadExternalAuthProvidersFromEnv(environ []string) ([]codersdk.ExternalAuthConfig, error) { diff --git a/cli/server_internal_test.go b/cli/server_internal_test.go index 66c9a66aeb..8150b97109 100644 --- a/cli/server_internal_test.go +++ b/cli/server_internal_test.go @@ -6,11 +6,17 @@ import ( "crypto/tls" "testing" + "github.com/spf13/pflag" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "cdr.dev/slog" "cdr.dev/slog/sloggers/sloghuman" + "cdr.dev/slog/sloggers/slogtest" + + "github.com/coder/coder/v2/cli/clibase" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/testutil" ) func Test_configureCipherSuites(t *testing.T) { @@ -169,3 +175,71 @@ func Test_configureCipherSuites(t *testing.T) { }) } } + +func TestRedirectHTTPToHTTPSDeprecation(t *testing.T) { + t.Parallel() + + testcases := []struct { + name string + environ clibase.Environ + flags []string + expected bool + }{ + { + name: "AllUnset", + environ: clibase.Environ{}, + flags: []string{}, + expected: false, + }, + { + name: "CODER_TLS_REDIRECT_HTTP=true", + environ: clibase.Environ{{Name: "CODER_TLS_REDIRECT_HTTP", Value: "true"}}, + flags: []string{}, + expected: true, + }, + { + name: "CODER_TLS_REDIRECT_HTTP_TO_HTTPS=true", + environ: clibase.Environ{{Name: "CODER_TLS_REDIRECT_HTTP_TO_HTTPS", Value: "true"}}, + flags: []string{}, + expected: true, + }, + { + name: "CODER_TLS_REDIRECT_HTTP=false", + environ: clibase.Environ{{Name: "CODER_TLS_REDIRECT_HTTP", Value: "false"}}, + flags: []string{}, + expected: false, + }, + { + name: "CODER_TLS_REDIRECT_HTTP_TO_HTTPS=false", + environ: clibase.Environ{{Name: "CODER_TLS_REDIRECT_HTTP_TO_HTTPS", Value: "false"}}, + flags: []string{}, + expected: false, + }, + { + name: "--tls-redirect-http-to-https", + environ: clibase.Environ{}, + flags: []string{"--tls-redirect-http-to-https"}, + expected: true, + }, + } + + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + logger := slogtest.Make(t, nil) + flags := pflag.NewFlagSet("test", pflag.ContinueOnError) + _ = flags.Bool("tls-redirect-http-to-https", true, "") + err := flags.Parse(tc.flags) + require.NoError(t, err) + inv := (&clibase.Invocation{Environ: tc.environ}).WithTestParsedFlags(t, flags) + cfg := &codersdk.DeploymentValues{} + opts := cfg.Options() + err = opts.SetDefaults() + require.NoError(t, err) + redirectHTTPToHTTPSDeprecation(ctx, logger, inv, cfg) + require.Equal(t, tc.expected, cfg.RedirectToAccessURL.Value()) + }) + } +} diff --git a/cli/ssh_internal_test.go b/cli/ssh_internal_test.go index 3e3e116e95..0630deb36d 100644 --- a/cli/ssh_internal_test.go +++ b/cli/ssh_internal_test.go @@ -5,16 +5,15 @@ import ( "net/url" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/xerrors" "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" - "github.com/coder/coder/v2/testutil" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/testutil" ) const ( diff --git a/docs/admin/configure.md b/docs/admin/configure.md index 7ec7faff77..527b73047f 100644 --- a/docs/admin/configure.md +++ b/docs/admin/configure.md @@ -29,7 +29,7 @@ export CODER_TLS_ENABLE=true export CODER_TLS_ADDRESS=0.0.0.0:443 ## Redirect from HTTP to HTTPS -export CODER_TLS_REDIRECT_HTTP=true +export CODER_REDIRECT_TO_ACCESS_URL=true # Start the Coder server coder server