diff --git a/cli/clitest/clitest.go b/cli/clitest/clitest.go index 20db312101..3e506a26b6 100644 --- a/cli/clitest/clitest.go +++ b/cli/clitest/clitest.go @@ -61,10 +61,10 @@ func NewWithCommand( t testing.TB, cmd *serpent.Command, args ...string, ) (*serpent.Invocation, config.Root) { configDir := config.Root(t.TempDir()) - // Keyring usage is disabled here because many existing tests expect the session token - // to be stored on disk and is not properly instrumented for parallel testing against - // the actual operating system keyring. - invArgs := append([]string{"--global-config", string(configDir), "--use-keyring=false"}, args...) + // Keyring usage is disabled here when --global-config is set because many existing + // tests expect the session token to be stored on disk and is not properly instrumented + // for parallel testing against the actual operating system keyring. + invArgs := append([]string{"--global-config", string(configDir)}, args...) return setupInvocation(t, cmd, invArgs...), configDir } diff --git a/cli/keyring_test.go b/cli/keyring_test.go index 27b7c12d53..7cb190845a 100644 --- a/cli/keyring_test.go +++ b/cli/keyring_test.go @@ -54,6 +54,7 @@ func setupKeyringTestEnv(t *testing.T, clientURL string, args ...string) keyring serviceName := keyringTestServiceName(t) root.WithKeyringServiceName(serviceName) + root.UseKeyringWithGlobalConfig() inv, cfg := clitest.NewWithDefaultKeyringCommand(t, cmd, args...) @@ -169,6 +170,7 @@ func TestUseKeyring(t *testing.T) { logoutCmd, err := logoutRoot.Command(logoutRoot.AGPL()) require.NoError(t, err) logoutRoot.WithKeyringServiceName(env.serviceName) + logoutRoot.UseKeyringWithGlobalConfig() logoutInv, _ := clitest.NewWithDefaultKeyringCommand(t, logoutCmd, "logout", diff --git a/cli/root.go b/cli/root.go index 2ec035d64a..7f0561e180 100644 --- a/cli/root.go +++ b/cli/root.go @@ -483,9 +483,9 @@ func (r *RootCmd) Command(subcommands []*serpent.Command) (*serpent.Command, err Flag: varUseKeyring, Env: envUseKeyring, Description: "Store and retrieve session tokens using the operating system " + - "keyring. Enabled by default. If the keyring is not supported on the " + - "current platform, file-based storage is used automatically. Set to " + - "false to force file-based storage.", + "keyring. This flag is ignored and file-based storage is used when " + + "--global-config is set or keyring usage is not supported on the current " + + "platform. Set to false to force file-based storage on supported platforms.", Default: "true", Value: serpent.BoolOf(&r.useKeyring), Group: globalGroup, @@ -536,11 +536,12 @@ type RootCmd struct { disableDirect bool debugHTTP bool - disableNetworkTelemetry bool - noVersionCheck bool - noFeatureWarning bool - useKeyring bool - keyringServiceName string + disableNetworkTelemetry bool + noVersionCheck bool + noFeatureWarning bool + useKeyring bool + keyringServiceName string + useKeyringWithGlobalConfig bool } // InitClient creates and configures a new client with authentication, telemetry, @@ -721,8 +722,14 @@ func (r *RootCmd) createUnauthenticatedClient(ctx context.Context, serverURL *ur // flag. func (r *RootCmd) ensureTokenBackend() sessionstore.Backend { if r.tokenBackend == nil { + // Checking for the --global-config directory being set is a bit wonky but necessary + // to allow extensions that invoke the CLI with this flag (e.g. VS code) to continue + // working without modification. In the future we should modify these extensions to + // either access the credential in the keyring (like Coder Desktop) or some other + // approach that doesn't rely on the session token being stored on disk. + assumeExtensionInUse := r.globalConfig != config.DefaultDir() && !r.useKeyringWithGlobalConfig keyringSupported := runtime.GOOS == "windows" || runtime.GOOS == "darwin" - if r.useKeyring && keyringSupported { + if r.useKeyring && !assumeExtensionInUse && keyringSupported { serviceName := sessionstore.DefaultServiceName if r.keyringServiceName != "" { serviceName = r.keyringServiceName @@ -742,6 +749,13 @@ func (r *RootCmd) WithKeyringServiceName(serviceName string) { r.keyringServiceName = serviceName } +// UseKeyringWithGlobalConfig enables the use of the keyring storage backend +// when the --global-config directory is set. This is only intended as an override +// for tests, which require specifying the global config directory for test isolation. +func (r *RootCmd) UseKeyringWithGlobalConfig() { + r.useKeyringWithGlobalConfig = true +} + type AgentAuth struct { // Agent Client config agentToken string diff --git a/cli/testdata/coder_--help.golden b/cli/testdata/coder_--help.golden index 0057fdc15d..ab13e2af71 100644 --- a/cli/testdata/coder_--help.golden +++ b/cli/testdata/coder_--help.golden @@ -111,9 +111,10 @@ variables or flags. --use-keyring bool, $CODER_USE_KEYRING (default: true) Store and retrieve session tokens using the operating system keyring. - Enabled by default. If the keyring is not supported on the current - platform, file-based storage is used automatically. Set to false to - force file-based storage. + This flag is ignored and file-based storage is used when + --global-config is set or keyring usage is not supported on the + current platform. Set to false to force file-based storage on + supported platforms. -v, --verbose bool, $CODER_VERBOSE Enable verbose output. diff --git a/docs/reference/cli/index.md b/docs/reference/cli/index.md index e22e2deeec..b26ec94a7f 100644 --- a/docs/reference/cli/index.md +++ b/docs/reference/cli/index.md @@ -179,7 +179,7 @@ Disable network telemetry. Network telemetry is collected when connecting to wor | Environment | $CODER_USE_KEYRING | | Default | true | -Store and retrieve session tokens using the operating system keyring. Enabled by default. If the keyring is not supported on the current platform, file-based storage is used automatically. Set to false to force file-based storage. +Store and retrieve session tokens using the operating system keyring. This flag is ignored and file-based storage is used when --global-config is set or keyring usage is not supported on the current platform. Set to false to force file-based storage on supported platforms. ### --global-config diff --git a/enterprise/cli/testdata/coder_--help.golden b/enterprise/cli/testdata/coder_--help.golden index 41ed320968..e199e8cc27 100644 --- a/enterprise/cli/testdata/coder_--help.golden +++ b/enterprise/cli/testdata/coder_--help.golden @@ -70,9 +70,10 @@ variables or flags. --use-keyring bool, $CODER_USE_KEYRING (default: true) Store and retrieve session tokens using the operating system keyring. - Enabled by default. If the keyring is not supported on the current - platform, file-based storage is used automatically. Set to false to - force file-based storage. + This flag is ignored and file-based storage is used when + --global-config is set or keyring usage is not supported on the + current platform. Set to false to force file-based storage on + supported platforms. -v, --verbose bool, $CODER_VERBOSE Enable verbose output.