cmd: propagate keyring reading errors

internal/clientcache/cmd/cache/addtoken.go

@@ -174,7 +174,7 @@ func (c *AddTokenCommand) Add(ctx context.Context, ui cli.Ui, apiClient *api.Cli
 
 		// Try to read the token from the keyring in a best effort way. Ignore
 		// any errors since the keyring may not be present on the system.
-		at := base.ReadTokenFromKeyring(ui, keyringType, tokenName)
+		at, _ := base.ReadTokenFromKeyring(ui, keyringType, tokenName)
 		if at != nil && (token == "" || pa.AuthTokenId == at.Id) {
 			pa.Keyring = &daemon.KeyringToken{
 				KeyringType: keyringType,

internal/clientcache/cmd/search/search_test.go

@@ -32,8 +32,8 @@ func (r *testCommander) Client(opt ...base.Option) (*api.Client, error) {
 	return client, nil
 }
 
-func (r *testCommander) ReadTokenFromKeyring(k, a string) *authtokens.AuthToken {
-	return r.at[a]
+func (r *testCommander) ReadTokenFromKeyring(k, a string) (*authtokens.AuthToken, error) {
+	return r.at[a], nil
 }
 
 func TestSearch(t *testing.T) {
@@ -50,7 +50,10 @@ func TestSearch(t *testing.T) {
 		Token:          "at_2_token",
 		ExpirationTime: time.Now().Add(time.Minute),
 	}
-	cmd := &testCommander{t: t, at: map[string]*authtokens.AuthToken{"tokenname": at, "unsupported": unsupportedAt}}
+	cmd := &testCommander{
+		t:  t,
+		at: map[string]*authtokens.AuthToken{"tokenname": at, "unsupported": unsupportedAt},
+	}
 	boundaryTokenReaderFn := func(ctx context.Context, addr, authToken string) (*authtokens.AuthToken, error) {
 		switch authToken {
 		case at.Token:
@@ -67,12 +70,16 @@ func TestSearch(t *testing.T) {
 	wg.Add(1)
 	go func() {
 		defer wg.Done()
-		srv.Serve(
+		err := srv.Serve(
 			t,
 			daemon.WithBoundaryTokenReaderFunc(ctx, boundaryTokenReaderFn),
 			daemon.WithReadyToServeNotificationCh(context.Background(), readyNotificationCh),
 		)
+		if err != nil {
+			t.Error("Failed to serve daemon:", err)
+		}
 	}()
+	t.Cleanup(wg.Wait)
 	<-readyNotificationCh
 	srv.AddKeyringToken(t, "address", "keyringtype", "tokenname", at.Id, boundaryTokenReaderFn)
 	srv.AddKeyringToken(t, "address", "keyringtype", "unsupported", unsupportedAt.Id, boundaryTokenReaderFn)

internal/clientcache/internal/cache/refresh.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/hashicorp/boundary/api"
 	"github.com/hashicorp/boundary/api/authtokens"
+	"github.com/hashicorp/boundary/internal/cmd/base"
 	"github.com/hashicorp/boundary/internal/errors"
 	"github.com/hashicorp/boundary/internal/event"
 	"github.com/hashicorp/boundary/internal/util"
@@ -82,8 +83,10 @@ func (r *RefreshService) cleanAndPickAuthTokens(ctx context.Context, u *user) (m
 			return nil, errors.Wrap(ctx, err, op, errors.WithMsg("for user %v, auth token %q", u, t.Id))
 		}
 		for _, kt := range keyringTokens {
-			at := r.repo.tokenKeyringFn(kt.KeyringType, kt.TokenName)
+			at, err := r.repo.tokenKeyringFn(kt.KeyringType, kt.TokenName)
 			switch {
+			case err != nil && !errors.Is(err, base.ErrNoToken):
+				event.WriteSysEvent(ctx, op, "failed to get token from keyring", "keyring", kt.KeyringType, "token name", kt.TokenName, "error", err)
 			case at == nil, at.Id != kt.AuthTokenId, at.UserId != t.UserId:
 				event.WriteSysEvent(ctx, op, "Removed keyring token since the keyring contents have changed since being cached", "keyring", kt.KeyringType, "token name", kt.TokenName, "old auth token id", kt.AuthTokenId)
 				// delete the keyring token if the auth token in the keyring

internal/clientcache/internal/cache/repository.go

@@ -20,7 +20,7 @@ const (
 )
 
 // KeyringTokenLookupFn takes a token name and returns the token from the keyring
-type KeyringTokenLookupFn func(keyring string, tokenName string) *authtokens.AuthToken
+type KeyringTokenLookupFn func(keyring string, tokenName string) (*authtokens.AuthToken, error)
 
 // BoundaryTokenReaderFn reads an auth token's resource information from boundary
 type BoundaryTokenReaderFn func(ctx context.Context, addr string, authToken string) (*authtokens.AuthToken, error)

internal/clientcache/internal/cache/repository_test.go

@@ -26,8 +26,8 @@ type ringToken struct {
 // mapBasedAuthTokenKeyringLookup provides a fake KeyringTokenLookupFn that uses
 // the provided map to perform lookups for the tokens
 func mapBasedAuthTokenKeyringLookup(m map[ringToken]*authtokens.AuthToken) KeyringTokenLookupFn {
-	return func(k, t string) *authtokens.AuthToken {
-		return m[ringToken{k, t}]
+	return func(k, t string) (*authtokens.AuthToken, error) {
+		return m[ringToken{k, t}], nil
 	}
 }
 

internal/clientcache/internal/cache/repository_token.go

@@ -196,7 +196,10 @@ func (r *Repository) AddKeyringToken(ctx context.Context, bAddr string, token Ke
 		return errors.New(ctx, errors.InvalidParameter, op, "boundary address is empty", errors.WithoutEvent())
 	}
 	kt := token.clone()
-	keyringStoredAt := r.tokenKeyringFn(kt.KeyringType, kt.TokenName)
+	keyringStoredAt, err := r.tokenKeyringFn(kt.KeyringType, kt.TokenName)
+	if err != nil {
+		return errors.Wrap(ctx, err, op, errors.WithMsg("unable to lookup token in keyring, keyring type: %q, token name: %q", kt.KeyringType, kt.TokenName), errors.WithoutEvent())
+	}
 	if keyringStoredAt == nil {
 		return errors.New(ctx, errors.InvalidParameter, op, "unable to find token in the keyring specified", errors.WithoutEvent())
 	}
@@ -224,7 +227,7 @@ func (r *Repository) AddKeyringToken(ctx context.Context, bAddr string, token Ke
 			at = keyringStoredAt
 		}
 	}
-	_, err := r.rw.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, func(reader db.Reader, writer db.Writer) error {
+	if _, err := r.rw.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, func(reader db.Reader, writer db.Writer) error {
 		if err := upsertUserAndAuthToken(ctx, reader, writer, bAddr, at); err != nil {
 			return errors.Wrap(ctx, err, op, errors.WithoutEvent())
 		}
@@ -236,8 +239,7 @@ func (r *Repository) AddKeyringToken(ctx context.Context, bAddr string, token Ke
 			return errors.Wrap(ctx, err, op, errors.WithoutEvent())
 		}
 		return nil
-	})
-	if err != nil {
+	}); err != nil {
 		return err
 	}
 

internal/clientcache/internal/daemon/server.go

@@ -48,7 +48,7 @@ const (
 // and retrieve the keyring and token information used by a command.
 type Commander interface {
 	ClientProvider
-	ReadTokenFromKeyring(keyringType, tokenName string) *authtokens.AuthToken
+	ReadTokenFromKeyring(keyringType, tokenName string) (*authtokens.AuthToken, error)
 }
 
 // ClientProvider is an interface that provides an api.Client

internal/clientcache/internal/daemon/token_handler_test.go

@@ -33,8 +33,8 @@ type ringToken struct {
 // mapBasedAuthTokenKeyringLookup provides a fake KeyringTokenLookupFn that uses
 // the provided map to perform lookups for the tokens
 func mapBasedAuthTokenKeyringLookup(m map[ringToken]*authtokens.AuthToken) cache.KeyringTokenLookupFn {
-	return func(k, t string) *authtokens.AuthToken {
-		return m[ringToken{k, t}]
+	return func(k, t string) (*authtokens.AuthToken, error) {
+		return m[ringToken{k, t}], nil
 	}
 }
 

internal/cmd/base/base.go

@@ -350,8 +350,10 @@ func (c *Command) Client(opt ...Option) (*api.Client, error) {
 			return nil, err
 		}
 
-		authToken := c.ReadTokenFromKeyring(keyringType, tokenName)
-		if authToken != nil {
+		authToken, err := c.ReadTokenFromKeyring(keyringType, tokenName)
+		if err != nil {
+			c.UI.Error(err.Error())
+		} else {
 			c.client.SetToken(authToken.Token)
 		}
 	}

internal/cmd/base/keyring.go

@@ -31,6 +31,9 @@ const (
 	PassPrefix       = "HashiCorp_Boundary"
 )
 
+// ErrNoToken is returned when no matching token could be found in the keyring
+var ErrNoToken = errors.New("no token found")
+
 func (c *Command) DiscoverKeyringTokenInfo() (string, string, error) {
 	// Stops the underlying library from invoking a dbus call that ends up
 	// freezing some systems
@@ -109,24 +112,26 @@ func (c *Command) DiscoverKeyringTokenInfo() (string, string, error) {
 	return keyringType, tokenName, nil
 }
 
-func ReadTokenFromKeyring(ui cli.Ui, keyringType, tokenName string) *authtokens.AuthToken {
+// ReadTokenFromKeyring will attempt to read the token with the name tokenName from the keyring
+// described by keyringType. It will return ErrNoToken if no token is found in the provided keyring.
+func ReadTokenFromKeyring(ui cli.Ui, keyringType, tokenName string) (*authtokens.AuthToken, error) {
+	const op = "base.ReadTokenFromKeyring"
 	var token string
 	var err error
 
 	switch keyringType {
 	case NoneKeyring:
-		return nil
+		return nil, ErrNoToken
 
 	case WincredKeyring, KeychainKeyring:
 		token, err = zkeyring.Get(StoredTokenName, tokenName)
 		if err != nil {
 			if err == zkeyring.ErrNotFound {
 				ui.Error("No saved credential found, continuing without")
 			} else {
-				ui.Error(fmt.Sprintf("Error reading auth token from keyring: %s", err))
 				ui.Warn("Token must be provided via BOUNDARY_TOKEN env var or -token flag. Reading the token can also be disabled via -keyring-type=none.")
+				return nil, fmt.Errorf("%s: error reading auth token from keyring: %w", op, err)
 			}
-			token = ""
 		}
 
 	default:
@@ -138,16 +143,14 @@ func ReadTokenFromKeyring(ui cli.Ui, keyringType, tokenName string) *authtokens.
 
 		kr, err := nkeyring.Open(krConfig)
 		if err != nil {
-			ui.Error(fmt.Sprintf("Error opening keyring: %s", err))
 			ui.Warn("Token must be provided via BOUNDARY_TOKEN env var or -token flag. Reading the token can also be disabled via -keyring-type=none.")
-			break
+			return nil, fmt.Errorf("%s: failed to open keyring: %w", op, err)
 		}
 
 		item, err := kr.Get(tokenName)
 		if err != nil {
-			ui.Error(fmt.Sprintf("Error fetching token from keyring: %s", err))
 			ui.Warn("Token must be provided via BOUNDARY_TOKEN env var or -token flag. Reading the token can also be disabled via -keyring-type=none.")
-			break
+			return nil, fmt.Errorf("%s: failed to get token from keyring: %w", op, err)
 		}
 
 		token = string(item.Data)
@@ -157,22 +160,24 @@ func ReadTokenFromKeyring(ui cli.Ui, keyringType, tokenName string) *authtokens.
 		tokenBytes, err := base64.RawStdEncoding.DecodeString(token)
 		switch {
 		case err != nil:
-			ui.Error(fmt.Errorf("Error base64-unmarshaling stored token from system credential store: %w", err).Error())
+			return nil, fmt.Errorf("error base64-unmarshaling stored token from system credential store: %w", err)
 		case len(tokenBytes) == 0:
-			ui.Error("Zero length token after decoding stored token from system credential store")
+			return nil, errors.New("zero length token after decoding stored token from system credential store")
 		default:
 			var authToken authtokens.AuthToken
 			if err := json.Unmarshal(tokenBytes, &authToken); err != nil {
-				ui.Error(fmt.Sprintf("Error unmarshaling stored token information after reading from system credential store: %s", err))
+				return nil, fmt.Errorf("error unmarshaling stored token information after reading from system credential store: %s", err)
 			} else {
-				return &authToken
+				return &authToken, nil
 			}
 		}
 	}
-	return nil
+	return nil, ErrNoToken
 }
 
-func (c *Command) ReadTokenFromKeyring(keyringType, tokenName string) *authtokens.AuthToken {
+// ReadTokenFromKeyring will attempt to read the token with the name tokenName from the keyring
+// described by keyringType. It will return ErrNoToken if no token is found in the provided keyring.
+func (c *Command) ReadTokenFromKeyring(keyringType, tokenName string) (*authtokens.AuthToken, error) {
 	return ReadTokenFromKeyring(c.UI, keyringType, tokenName)
 }
 

internal/cmd/commands/config/token.go

@@ -136,7 +136,11 @@ func (c *TokenCommand) Run(args []string) (ret int) {
 			c.UI.Error(err.Error())
 			return base.CommandCliError
 		}
-		authToken = c.ReadTokenFromKeyring(keyringType, tokenName)
+		authToken, err = c.ReadTokenFromKeyring(keyringType, tokenName)
+		if err != nil {
+			c.UI.Error(err.Error())
+			return base.CommandCliError
+		}
 	} else {
 		// Fallback to env/CLI but we can only get just the token value this way, at
 		// least for now