diff --git a/internal/client/client.go b/internal/client/client.go index b236267..5cdc72d 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -54,7 +54,7 @@ func newClientForCurrentContext(cmd *cobra.Command) (Client, error) { } func newClientForContext(cmd *cobra.Command, contextName string, secretStore storage.SecretStore) (*authzed.Client, error) { - currentToken, err := storage.GetToken(contextName, secretStore) + currentToken, err := storage.GetTokenIfExists(contextName, secretStore) if err != nil { return nil, err } @@ -74,6 +74,14 @@ func newClientForContext(cmd *cobra.Command, contextName string, secretStore sto // GetCurrentTokenWithCLIOverride returns the current token, but overridden by any parameter specified via CLI args func GetCurrentTokenWithCLIOverride(cmd *cobra.Command, configStore storage.ConfigStore, secretStore storage.SecretStore) (storage.Token, error) { + // Handle the no-config case separately + configExists, err := configStore.Exists() + if err != nil { + return storage.Token{}, err + } + if !configExists { + return GetTokenWithCLIOverride(cmd, storage.Token{}) + } token, err := storage.CurrentToken( configStore, secretStore, diff --git a/internal/client/client_test.go b/internal/client/client_test.go index e1a0d92..8770647 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -2,6 +2,7 @@ package client_test import ( "os" + "path" "testing" "github.com/authzed/zed/internal/client" @@ -12,10 +13,11 @@ import ( ) func TestGetTokenWithCLIOverride(t *testing.T) { + require := require.New(t) testCert, err := os.CreateTemp("", "") - require.NoError(t, err) + require.NoError(err) _, err = testCert.Write([]byte("hi")) - require.NoError(t, err) + require.NoError(err) cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, zedtesting.StringFlag{FlagName: "token", FlagValue: "t1", Changed: true}, zedtesting.StringFlag{FlagName: "certificate-path", FlagValue: testCert.Name(), Changed: true}, @@ -29,13 +31,13 @@ func TestGetTokenWithCLIOverride(t *testing.T) { // cli args take precedence when defined to, err := client.GetTokenWithCLIOverride(cmd, storage.Token{}) - require.NoError(t, err) - require.True(t, to.AnyValue()) - require.Equal(t, "t1", to.APIToken) - require.Equal(t, "e1", to.Endpoint) - require.Equal(t, []byte("hi"), to.CACert) - require.Equal(t, &bTrue, to.Insecure) - require.Equal(t, &bTrue, to.NoVerifyCA) + require.NoError(err) + require.True(to.AnyValue()) + require.Equal("t1", to.APIToken) + require.Equal("e1", to.Endpoint) + require.Equal([]byte("hi"), to.CACert) + require.Equal(&bTrue, to.Insecure) + require.Equal(&bTrue, to.NoVerifyCA) // storage token takes precedence when defined cmd = zedtesting.CreateTestCobraCommandWithFlagValue(t, @@ -52,11 +54,66 @@ func TestGetTokenWithCLIOverride(t *testing.T) { Insecure: &bFalse, NoVerifyCA: &bFalse, }) - require.NoError(t, err) - require.True(t, to.AnyValue()) - require.Equal(t, "t2", to.APIToken) - require.Equal(t, "e2", to.Endpoint) - require.Equal(t, []byte("bye"), to.CACert) - require.Equal(t, &bFalse, to.Insecure) - require.Equal(t, &bFalse, to.NoVerifyCA) + require.NoError(err) + require.True(to.AnyValue()) + require.Equal("t2", to.APIToken) + require.Equal("e2", to.Endpoint) + require.Equal([]byte("bye"), to.CACert) + require.Equal(&bFalse, to.Insecure) + require.Equal(&bFalse, to.NoVerifyCA) +} + +func TestGetCurrentTokenWithCLIOverrideWithoutConfigFile(t *testing.T) { + // When we refactored the token setting logic, we broke the workflow where zed is used without a saved + // configuration. This asserts that that workflow works. + require := require.New(t) + cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, + zedtesting.StringFlag{FlagName: "token", FlagValue: "t1", Changed: true}, + zedtesting.StringFlag{FlagName: "endpoint", FlagValue: "e1", Changed: true}, + zedtesting.StringFlag{FlagName: "certificate-path", FlagValue: "", Changed: false}, + zedtesting.BoolFlag{FlagName: "insecure", FlagValue: true, Changed: true}, + ) + + configStore := &storage.JSONConfigStore{ConfigPath: "/not/a/valid/path"} + secretStore := &storage.KeychainSecretStore{ConfigPath: "/not/a/valid/path"} + token, err := client.GetCurrentTokenWithCLIOverride(cmd, configStore, secretStore) + + // cli args take precedence when defined + require.NoError(err) + require.True(token.AnyValue()) + require.Equal("t1", token.APIToken) + require.Equal("e1", token.Endpoint) + require.NotNil(token.Insecure) + require.True(*token.Insecure) +} + +func TestGetCurrentTokenWithCLIOverrideWithoutSecretFile(t *testing.T) { + // When we refactored the token setting logic, we broke the workflow where zed is used without a saved + // context. This asserts that that workflow works. + require := require.New(t) + cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, + zedtesting.StringFlag{FlagName: "token", FlagValue: "t1", Changed: true}, + zedtesting.StringFlag{FlagName: "endpoint", FlagValue: "e1", Changed: true}, + zedtesting.StringFlag{FlagName: "certificate-path", FlagValue: "", Changed: false}, + zedtesting.BoolFlag{FlagName: "insecure", FlagValue: true, Changed: true}, + ) + + bTrue := true + + tmpDir, err := os.MkdirTemp("", "") + require.NoError(err) + configPath := path.Join(tmpDir, "config.json") + err = os.WriteFile(configPath, []byte("{}"), 0o600) + require.NoError(err) + + configStore := &storage.JSONConfigStore{ConfigPath: tmpDir} + secretStore := &storage.KeychainSecretStore{ConfigPath: "/not/a/valid/path"} + token, err := client.GetCurrentTokenWithCLIOverride(cmd, configStore, secretStore) + + // cli args take precedence when defined + require.NoError(err) + require.True(token.AnyValue()) + require.Equal("t1", token.APIToken) + require.Equal("e1", token.Endpoint) + require.Equal(&bTrue, token.Insecure) } diff --git a/internal/storage/config.go b/internal/storage/config.go index 739079e..1fca679 100644 --- a/internal/storage/config.go +++ b/internal/storage/config.go @@ -3,6 +3,7 @@ package storage import ( "encoding/json" "errors" + "io/fs" "os" "path/filepath" "runtime" @@ -15,6 +16,9 @@ const configFileName = "config.json" // ErrConfigNotFound is returned if there is no Config in a ConfigStore. var ErrConfigNotFound = errors.New("config did not exist") +// ErrTokenNotFound is returned if there is no Token in a ConfigStore. +var ErrTokenNotFound = errors.New("token does not exist") + // Config represents the contents of a zed configuration file. type Config struct { Version string @@ -25,6 +29,7 @@ type Config struct { type ConfigStore interface { Get() (Config, error) Put(Config) error + Exists() (bool, error) } // TokenWithOverride returns a Token that retrieves its values from the reference Token, and has its values overridden @@ -69,23 +74,28 @@ func TokenWithOverride(overrideToken Token, referenceToken Token) (Token, error) // CurrentToken is a convenient way to obtain the CurrentToken field from the // current Config. -func CurrentToken(cs ConfigStore, ss SecretStore) (Token, error) { +func CurrentToken(cs ConfigStore, ss SecretStore) (token Token, err error) { cfg, err := cs.Get() if err != nil { return Token{}, err } - return GetToken(cfg.CurrentToken, ss) + return GetTokenIfExists(cfg.CurrentToken, ss) } // SetCurrentToken is a convenient way to set the CurrentToken field in a // the current config. func SetCurrentToken(name string, cs ConfigStore, ss SecretStore) error { // Ensure the token exists - if _, err := GetToken(name, ss); err != nil { + exists, err := TokenExists(name, ss) + if err != nil { return err } + if !exists { + return ErrTokenNotFound + } + cfg, err := cs.Get() if err != nil { if errors.Is(err, ErrConfigNotFound) { @@ -110,10 +120,9 @@ var _ ConfigStore = JSONConfigStore{} // Get parses a Config from the filesystem. func (s JSONConfigStore) Get() (Config, error) { cfgBytes, err := os.ReadFile(filepath.Join(s.ConfigPath, configFileName)) - if err != nil { - if os.IsNotExist(err) { - return Config{}, ErrConfigNotFound - } + if errors.Is(err, fs.ErrNotExist) { + return Config{}, ErrConfigNotFound + } else if err != nil { return Config{}, err } @@ -139,6 +148,15 @@ func (s JSONConfigStore) Put(cfg Config) error { return atomicWriteFile(filepath.Join(s.ConfigPath, configFileName), cfgBytes, 0o774) } +func (s JSONConfigStore) Exists() (bool, error) { + if _, err := os.Stat(filepath.Join(s.ConfigPath, configFileName)); errors.Is(err, fs.ErrNotExist) { + return false, nil + } else if err != nil { + return false, err + } + return true, nil +} + // atomicWriteFile writes data to filename+some suffix, then renames it into // filename. // diff --git a/internal/storage/secrets.go b/internal/storage/secrets.go index 90d1ebb..c76620d 100644 --- a/internal/storage/secrets.go +++ b/internal/storage/secrets.go @@ -14,9 +14,6 @@ import ( "github.com/authzed/zed/internal/console" ) -// ErrTokenNotFound is returned if there is no Token in a ConfigStore. -var ErrTokenNotFound = errors.New("token does not exist") - type Token struct { Name string Endpoint string @@ -72,7 +69,8 @@ type SecretStore interface { Put(s Secrets) error } -func GetToken(name string, ss SecretStore) (Token, error) { +// Returns an empty token if no token exists. +func GetTokenIfExists(name string, ss SecretStore) (Token, error) { secrets, err := ss.Get() if err != nil { return Token{}, err @@ -84,7 +82,22 @@ func GetToken(name string, ss SecretStore) (Token, error) { } } - return Token{}, ErrTokenNotFound + return Token{}, nil +} + +func TokenExists(name string, ss SecretStore) (bool, error) { + secrets, err := ss.Get() + if err != nil { + return false, err + } + + for _, token := range secrets.Tokens { + if name == token.Name { + return true, nil + } + } + + return false, nil } func PutToken(t Token, ss SecretStore) error {