From bccc2d621486da7baf067e43e33dddf55c578242 Mon Sep 17 00:00:00 2001 From: Asher Date: Fri, 11 Aug 2023 14:20:50 -0800 Subject: [PATCH 1/3] Add header-process flag This allows specifying a command to run that can output headers for cases where users require dynamic headers (like to authenticate to their VPN). The primary use case is to add this flag in SSH configs created by the VS Code plugin, although maybe config-ssh should do the same. --- cli/login.go | 2 +- cli/root.go | 43 ++++++++++++++++++--- cli/root_test.go | 23 ++++++++--- cli/testdata/coder_--help.golden | 4 ++ cli/vscodessh.go | 2 +- docs/cli.md | 9 +++++ enterprise/cli/testdata/coder_--help.golden | 4 ++ 7 files changed, 74 insertions(+), 13 deletions(-) diff --git a/cli/login.go b/cli/login.go index e16118dfec0d6..2fa7eb231ce9c 100644 --- a/cli/login.go +++ b/cli/login.go @@ -76,7 +76,7 @@ func (r *RootCmd) login() *clibase.Cmd { serverURL.Scheme = "https" } - client, err := r.createUnauthenticatedClient(serverURL) + client, err := r.createUnauthenticatedClient(ctx, serverURL) if err != nil { return err } diff --git a/cli/root.go b/cli/root.go index 036be18a01300..4e212688593ab 100644 --- a/cli/root.go +++ b/cli/root.go @@ -13,6 +13,7 @@ import ( "net/http" "net/url" "os" + "os/exec" "os/signal" "path/filepath" "runtime" @@ -55,6 +56,7 @@ const ( varAgentToken = "agent-token" varAgentURL = "agent-url" varHeader = "header" + varHeaderProcess = "header-process" varNoOpen = "no-open" varNoVersionCheck = "no-version-warning" varNoFeatureWarning = "no-feature-warning" @@ -356,6 +358,13 @@ func (r *RootCmd) Command(subcommands []*clibase.Cmd) (*clibase.Cmd, error) { Value: clibase.StringArrayOf(&r.header), Group: globalGroup, }, + { + Flag: varHeaderProcess, + Env: "CODER_HEADER_PROCESS", + Description: "An external process that outputs JSON-encoded key-value pairs to be used as additional HTTP headers added to all requests.", + Value: clibase.StringOf(&r.headerProcess), + Group: globalGroup, + }, { Flag: varNoOpen, Env: "CODER_NO_OPEN", @@ -437,6 +446,7 @@ type RootCmd struct { token string globalConfig string header []string + headerProcess string agentToken string agentURL *url.URL forceTTY bool @@ -540,9 +550,7 @@ func (r *RootCmd) initClientInternal(client *codersdk.Client, allowTokenMissing return err } } - err = r.setClient( - client, r.clientURL, - ) + err = r.setClient(inv.Context(), client, r.clientURL) if err != nil { return err } @@ -592,7 +600,7 @@ func (r *RootCmd) initClientInternal(client *codersdk.Client, allowTokenMissing } } -func (r *RootCmd) setClient(client *codersdk.Client, serverURL *url.URL) error { +func (r *RootCmd) setClient(ctx context.Context, client *codersdk.Client, serverURL *url.URL) error { transport := &headerTransport{ transport: http.DefaultTransport, header: http.Header{}, @@ -604,6 +612,29 @@ func (r *RootCmd) setClient(client *codersdk.Client, serverURL *url.URL) error { } transport.header.Add(parts[0], parts[1]) } + if r.headerProcess != "" { + shell := "sh" + caller := "-c" + if runtime.GOOS == "windows" { + shell = "cmd.exe" + caller = "/c" + } + // #nosec + cmd := exec.CommandContext(ctx, shell, caller, r.headerProcess) + cmd.Env = append(os.Environ(), "CODER_URL="+serverURL.String()) + out, err := cmd.Output() + if err != nil { + return xerrors.Errorf("failed to run %v (out: %q): %w", cmd.Args, out, err) + } + var headers map[string]string + err = json.Unmarshal(out, &headers) + if err != nil { + return xerrors.Errorf("failed to parse json from %v (out: %q): %w", cmd.Args, out, err) + } + for key, value := range headers { + transport.header.Add(key, value) + } + } client.URL = serverURL client.HTTPClient = &http.Client{ Transport: transport, @@ -611,9 +642,9 @@ func (r *RootCmd) setClient(client *codersdk.Client, serverURL *url.URL) error { return nil } -func (r *RootCmd) createUnauthenticatedClient(serverURL *url.URL) (*codersdk.Client, error) { +func (r *RootCmd) createUnauthenticatedClient(ctx context.Context, serverURL *url.URL) (*codersdk.Client, error) { var client codersdk.Client - err := r.setClient(&client, serverURL) + err := r.setClient(ctx, &client, serverURL) return &client, err } diff --git a/cli/root_test.go b/cli/root_test.go index c892701a3acbc..70118a726890d 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "runtime" "strings" "sync/atomic" "testing" @@ -72,20 +73,28 @@ func TestRoot(t *testing.T) { t.Run("Header", func(t *testing.T) { t.Parallel() + var url string var called int64 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { atomic.AddInt64(&called, 1) assert.Equal(t, "wow", r.Header.Get("X-Testing")) assert.Equal(t, "Dean was Here!", r.Header.Get("Cool-Header")) + assert.Equal(t, "very-wow-"+url, r.Header.Get("X-Process-Testing")) w.WriteHeader(http.StatusGone) })) defer srv.Close() + url = srv.URL buf := new(bytes.Buffer) + coderURLEnv := "$CODER_URL" + if runtime.GOOS == "windows" { + coderURLEnv = "%CODER_URL%" + } inv, _ := clitest.New(t, "--no-feature-warning", "--no-version-warning", "--header", "X-Testing=wow", "--header", "Cool-Header=Dean was Here!", + "--header-process", "printf '{\"X-Process-Testing\": \"very-wow-'"+coderURLEnv+"'\"}'", "login", srv.URL, ) inv.Stdout = buf @@ -97,8 +106,8 @@ func TestRoot(t *testing.T) { }) } -// TestDERPHeaders ensures that the client sends the global `--header`s to the -// DERP server when connecting. +// TestDERPHeaders ensures that the client sends the global `--header`s and +// `--header-process` to the DERP server when connecting. func TestDERPHeaders(t *testing.T) { t.Parallel() @@ -129,8 +138,9 @@ func TestDERPHeaders(t *testing.T) { // Inject custom /derp handler so we can inspect the headers. var ( expectedHeaders = map[string]string{ - "X-Test-Header": "test-value", - "Cool-Header": "Dean was Here!", + "X-Test-Header": "test-value", + "Cool-Header": "Dean was Here!", + "X-Process-Testing": "very-wow", } derpCalled int64 ) @@ -159,9 +169,12 @@ func TestDERPHeaders(t *testing.T) { "--no-version-warning", "ping", workspace.Name, "-n", "1", + "--header-process", "printf '{\"X-Process-Testing\": \"very-wow\"}'", } for k, v := range expectedHeaders { - args = append(args, "--header", fmt.Sprintf("%s=%s", k, v)) + if k != "X-Process-Testing" { + args = append(args, "--header", fmt.Sprintf("%s=%s", k, v)) + } } inv, root := clitest.New(t, args...) clitest.SetupConfig(t, client, root) diff --git a/cli/testdata/coder_--help.golden b/cli/testdata/coder_--help.golden index e074756680e84..d2853f1876017 100644 --- a/cli/testdata/coder_--help.golden +++ b/cli/testdata/coder_--help.golden @@ -62,6 +62,10 @@ variables or flags. Additional HTTP headers added to all requests. Provide as key=value. Can be specified multiple times. + --header-process string, $CODER_HEADER_PROCESS + An external process that outputs JSON-encoded key-value pairs to be + used as additional HTTP headers added to all requests. + --no-feature-warning bool, $CODER_NO_FEATURE_WARNING Suppress warnings about unlicensed features. diff --git a/cli/vscodessh.go b/cli/vscodessh.go index 136a0d727c17a..7a576581fa05c 100644 --- a/cli/vscodessh.go +++ b/cli/vscodessh.go @@ -86,7 +86,7 @@ func (r *RootCmd) vscodeSSH() *clibase.Cmd { client.SetSessionToken(string(sessionToken)) // This adds custom headers to the request! - err = r.setClient(client, serverURL) + err = r.setClient(ctx, client, serverURL) if err != nil { return xerrors.Errorf("set client: %w", err) } diff --git a/docs/cli.md b/docs/cli.md index 1a0c40a22fca6..38a5c8e968e26 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -96,6 +96,15 @@ Path to the global `coder` config directory. Additional HTTP headers added to all requests. Provide as key=value. Can be specified multiple times. +### --header-process + +| | | +| ----------- | ---------------------------------- | +| Type | string | +| Environment | $CODER_HEADER_PROCESS | + +An external process that outputs JSON-encoded key-value pairs to be used as additional HTTP headers added to all requests. + ### --no-feature-warning | | | diff --git a/enterprise/cli/testdata/coder_--help.golden b/enterprise/cli/testdata/coder_--help.golden index 7fc6962ded847..deb0319a61215 100644 --- a/enterprise/cli/testdata/coder_--help.golden +++ b/enterprise/cli/testdata/coder_--help.golden @@ -33,6 +33,10 @@ variables or flags. Additional HTTP headers added to all requests. Provide as key=value. Can be specified multiple times. + --header-process string, $CODER_HEADER_PROCESS + An external process that outputs JSON-encoded key-value pairs to be + used as additional HTTP headers added to all requests. + --no-feature-warning bool, $CODER_NO_FEATURE_WARNING Suppress warnings about unlicensed features. From 4a439ce775388f561738feb6fdf47a606a60c511 Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 14 Aug 2023 09:46:50 -0800 Subject: [PATCH 2/3] Rename header-process to header-command --- cli/root.go | 14 +++++++------- cli/root_test.go | 6 +++--- cli/testdata/coder_--help.golden | 4 ++-- docs/cli.md | 6 +++--- enterprise/cli/testdata/coder_--help.golden | 4 ++-- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/cli/root.go b/cli/root.go index 4e212688593ab..b69d6d77bf886 100644 --- a/cli/root.go +++ b/cli/root.go @@ -56,7 +56,7 @@ const ( varAgentToken = "agent-token" varAgentURL = "agent-url" varHeader = "header" - varHeaderProcess = "header-process" + varHeaderCommand = "header-command" varNoOpen = "no-open" varNoVersionCheck = "no-version-warning" varNoFeatureWarning = "no-feature-warning" @@ -359,10 +359,10 @@ func (r *RootCmd) Command(subcommands []*clibase.Cmd) (*clibase.Cmd, error) { Group: globalGroup, }, { - Flag: varHeaderProcess, - Env: "CODER_HEADER_PROCESS", + Flag: varHeaderCommand, + Env: "CODER_HEADER_COMMAND", Description: "An external process that outputs JSON-encoded key-value pairs to be used as additional HTTP headers added to all requests.", - Value: clibase.StringOf(&r.headerProcess), + Value: clibase.StringOf(&r.headerCommand), Group: globalGroup, }, { @@ -446,7 +446,7 @@ type RootCmd struct { token string globalConfig string header []string - headerProcess string + headerCommand string agentToken string agentURL *url.URL forceTTY bool @@ -612,7 +612,7 @@ func (r *RootCmd) setClient(ctx context.Context, client *codersdk.Client, server } transport.header.Add(parts[0], parts[1]) } - if r.headerProcess != "" { + if r.headerCommand != "" { shell := "sh" caller := "-c" if runtime.GOOS == "windows" { @@ -620,7 +620,7 @@ func (r *RootCmd) setClient(ctx context.Context, client *codersdk.Client, server caller = "/c" } // #nosec - cmd := exec.CommandContext(ctx, shell, caller, r.headerProcess) + cmd := exec.CommandContext(ctx, shell, caller, r.headerCommand) cmd.Env = append(os.Environ(), "CODER_URL="+serverURL.String()) out, err := cmd.Output() if err != nil { diff --git a/cli/root_test.go b/cli/root_test.go index 70118a726890d..42f7517bb1d24 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -94,7 +94,7 @@ func TestRoot(t *testing.T) { "--no-version-warning", "--header", "X-Testing=wow", "--header", "Cool-Header=Dean was Here!", - "--header-process", "printf '{\"X-Process-Testing\": \"very-wow-'"+coderURLEnv+"'\"}'", + "--header-command", "printf '{\"X-Process-Testing\": \"very-wow-'"+coderURLEnv+"'\"}'", "login", srv.URL, ) inv.Stdout = buf @@ -107,7 +107,7 @@ func TestRoot(t *testing.T) { } // TestDERPHeaders ensures that the client sends the global `--header`s and -// `--header-process` to the DERP server when connecting. +// `--header-command` to the DERP server when connecting. func TestDERPHeaders(t *testing.T) { t.Parallel() @@ -169,7 +169,7 @@ func TestDERPHeaders(t *testing.T) { "--no-version-warning", "ping", workspace.Name, "-n", "1", - "--header-process", "printf '{\"X-Process-Testing\": \"very-wow\"}'", + "--header-command", "printf '{\"X-Process-Testing\": \"very-wow\"}'", } for k, v := range expectedHeaders { if k != "X-Process-Testing" { diff --git a/cli/testdata/coder_--help.golden b/cli/testdata/coder_--help.golden index d2853f1876017..27fb6dd15e449 100644 --- a/cli/testdata/coder_--help.golden +++ b/cli/testdata/coder_--help.golden @@ -62,8 +62,8 @@ variables or flags. Additional HTTP headers added to all requests. Provide as key=value. Can be specified multiple times. - --header-process string, $CODER_HEADER_PROCESS - An external process that outputs JSON-encoded key-value pairs to be + --header-command string, $CODER_HEADER_COMMAND + An external command that outputs JSON-encoded key-value pairs to be used as additional HTTP headers added to all requests. --no-feature-warning bool, $CODER_NO_FEATURE_WARNING diff --git a/docs/cli.md b/docs/cli.md index 38a5c8e968e26..937fcc7b827df 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -96,14 +96,14 @@ Path to the global `coder` config directory. Additional HTTP headers added to all requests. Provide as key=value. Can be specified multiple times. -### --header-process +### --header-command | | | | ----------- | ---------------------------------- | | Type | string | -| Environment | $CODER_HEADER_PROCESS | +| Environment | $CODER_HEADER_COMMAND | -An external process that outputs JSON-encoded key-value pairs to be used as additional HTTP headers added to all requests. +An external command that outputs JSON-encoded key-value pairs to be used as additional HTTP headers added to all requests. ### --no-feature-warning diff --git a/enterprise/cli/testdata/coder_--help.golden b/enterprise/cli/testdata/coder_--help.golden index deb0319a61215..adb8d78fa1947 100644 --- a/enterprise/cli/testdata/coder_--help.golden +++ b/enterprise/cli/testdata/coder_--help.golden @@ -33,8 +33,8 @@ variables or flags. Additional HTTP headers added to all requests. Provide as key=value. Can be specified multiple times. - --header-process string, $CODER_HEADER_PROCESS - An external process that outputs JSON-encoded key-value pairs to be + --header-command string, $CODER_HEADER_COMMAND + An external command that outputs JSON-encoded key-value pairs to be used as additional HTTP headers added to all requests. --no-feature-warning bool, $CODER_NO_FEATURE_WARNING From 958c6c4301c662a0ef9f7e43b233692d73964a57 Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 14 Aug 2023 10:41:08 -0800 Subject: [PATCH 3/3] Make header-command newline-delimited Instead of JSON. --- cli/root.go | 37 ++++++++++++--------- cli/root_test.go | 5 +-- cli/testdata/coder_--help.golden | 5 +-- docs/cli.md | 2 +- enterprise/cli/testdata/coder_--help.golden | 5 +-- 5 files changed, 31 insertions(+), 23 deletions(-) diff --git a/cli/root.go b/cli/root.go index b69d6d77bf886..3197a3e3fce21 100644 --- a/cli/root.go +++ b/cli/root.go @@ -1,6 +1,8 @@ package cli import ( + "bufio" + "bytes" "context" "encoding/base64" "encoding/json" @@ -361,7 +363,7 @@ func (r *RootCmd) Command(subcommands []*clibase.Cmd) (*clibase.Cmd, error) { { Flag: varHeaderCommand, Env: "CODER_HEADER_COMMAND", - Description: "An external process that outputs JSON-encoded key-value pairs to be used as additional HTTP headers added to all requests.", + Description: "An external command that outputs additional HTTP headers added to all requests. The command must output each header as `key=value` on its own line.", Value: clibase.StringOf(&r.headerCommand), Group: globalGroup, }, @@ -605,13 +607,7 @@ func (r *RootCmd) setClient(ctx context.Context, client *codersdk.Client, server transport: http.DefaultTransport, header: http.Header{}, } - for _, header := range r.header { - parts := strings.SplitN(header, "=", 2) - if len(parts) < 2 { - return xerrors.Errorf("split header %q had less than two parts", header) - } - transport.header.Add(parts[0], parts[1]) - } + headers := r.header if r.headerCommand != "" { shell := "sh" caller := "-c" @@ -619,22 +615,31 @@ func (r *RootCmd) setClient(ctx context.Context, client *codersdk.Client, server shell = "cmd.exe" caller = "/c" } + var outBuf bytes.Buffer // #nosec cmd := exec.CommandContext(ctx, shell, caller, r.headerCommand) cmd.Env = append(os.Environ(), "CODER_URL="+serverURL.String()) - out, err := cmd.Output() + cmd.Stdout = &outBuf + cmd.Stderr = io.Discard + err := cmd.Run() if err != nil { - return xerrors.Errorf("failed to run %v (out: %q): %w", cmd.Args, out, err) + return xerrors.Errorf("failed to run %v: %w", cmd.Args, err) } - var headers map[string]string - err = json.Unmarshal(out, &headers) - if err != nil { - return xerrors.Errorf("failed to parse json from %v (out: %q): %w", cmd.Args, out, err) + scanner := bufio.NewScanner(&outBuf) + for scanner.Scan() { + headers = append(headers, scanner.Text()) } - for key, value := range headers { - transport.header.Add(key, value) + if err := scanner.Err(); err != nil { + return xerrors.Errorf("scan %v: %w", cmd.Args, err) } } + for _, header := range headers { + parts := strings.SplitN(header, "=", 2) + if len(parts) < 2 { + return xerrors.Errorf("split header %q had less than two parts", header) + } + transport.header.Add(parts[0], parts[1]) + } client.URL = serverURL client.HTTPClient = &http.Client{ Transport: transport, diff --git a/cli/root_test.go b/cli/root_test.go index 42f7517bb1d24..fb19aae6884a8 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -80,6 +80,7 @@ func TestRoot(t *testing.T) { assert.Equal(t, "wow", r.Header.Get("X-Testing")) assert.Equal(t, "Dean was Here!", r.Header.Get("Cool-Header")) assert.Equal(t, "very-wow-"+url, r.Header.Get("X-Process-Testing")) + assert.Equal(t, "more-wow", r.Header.Get("X-Process-Testing2")) w.WriteHeader(http.StatusGone) })) defer srv.Close() @@ -94,7 +95,7 @@ func TestRoot(t *testing.T) { "--no-version-warning", "--header", "X-Testing=wow", "--header", "Cool-Header=Dean was Here!", - "--header-command", "printf '{\"X-Process-Testing\": \"very-wow-'"+coderURLEnv+"'\"}'", + "--header-command", "printf X-Process-Testing=very-wow-"+coderURLEnv+"'\\r\\n'X-Process-Testing2=more-wow", "login", srv.URL, ) inv.Stdout = buf @@ -169,7 +170,7 @@ func TestDERPHeaders(t *testing.T) { "--no-version-warning", "ping", workspace.Name, "-n", "1", - "--header-command", "printf '{\"X-Process-Testing\": \"very-wow\"}'", + "--header-command", "printf X-Process-Testing=very-wow", } for k, v := range expectedHeaders { if k != "X-Process-Testing" { diff --git a/cli/testdata/coder_--help.golden b/cli/testdata/coder_--help.golden index 27fb6dd15e449..6e988a9f568fd 100644 --- a/cli/testdata/coder_--help.golden +++ b/cli/testdata/coder_--help.golden @@ -63,8 +63,9 @@ variables or flags. Can be specified multiple times. --header-command string, $CODER_HEADER_COMMAND - An external command that outputs JSON-encoded key-value pairs to be - used as additional HTTP headers added to all requests. + An external command that outputs additional HTTP headers added to all + requests. The command must output each header as `key=value` on its + own line. --no-feature-warning bool, $CODER_NO_FEATURE_WARNING Suppress warnings about unlicensed features. diff --git a/docs/cli.md b/docs/cli.md index 937fcc7b827df..96acc1cf940db 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -103,7 +103,7 @@ Additional HTTP headers added to all requests. Provide as key=value. Can be spec | Type | string | | Environment | $CODER_HEADER_COMMAND | -An external command that outputs JSON-encoded key-value pairs to be used as additional HTTP headers added to all requests. +An external command that outputs additional HTTP headers added to all requests. The command must output each header as `key=value` on its own line. ### --no-feature-warning diff --git a/enterprise/cli/testdata/coder_--help.golden b/enterprise/cli/testdata/coder_--help.golden index adb8d78fa1947..ae24592079a69 100644 --- a/enterprise/cli/testdata/coder_--help.golden +++ b/enterprise/cli/testdata/coder_--help.golden @@ -34,8 +34,9 @@ variables or flags. Can be specified multiple times. --header-command string, $CODER_HEADER_COMMAND - An external command that outputs JSON-encoded key-value pairs to be - used as additional HTTP headers added to all requests. + An external command that outputs additional HTTP headers added to all + requests. The command must output each header as `key=value` on its + own line. --no-feature-warning bool, $CODER_NO_FEATURE_WARNING Suppress warnings about unlicensed features.