diff --git a/internal/clientcache/cmd/cache/addtoken.go b/internal/clientcache/cmd/cache/addtoken.go index 62d858673f..5bc20060d7 100644 --- a/internal/clientcache/cmd/cache/addtoken.go +++ b/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, diff --git a/internal/clientcache/cmd/search/search_test.go b/internal/clientcache/cmd/search/search_test.go index 1e5e134df2..3b02507de1 100644 --- a/internal/clientcache/cmd/search/search_test.go +++ b/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) diff --git a/internal/clientcache/internal/cache/refresh.go b/internal/clientcache/internal/cache/refresh.go index a7193ac963..aff0febddd 100644 --- a/internal/clientcache/internal/cache/refresh.go +++ b/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 diff --git a/internal/clientcache/internal/cache/repository.go b/internal/clientcache/internal/cache/repository.go index eb5883df5b..3140ab68aa 100644 --- a/internal/clientcache/internal/cache/repository.go +++ b/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) diff --git a/internal/clientcache/internal/cache/repository_test.go b/internal/clientcache/internal/cache/repository_test.go index 81efc1b6d6..c4cab8b328 100644 --- a/internal/clientcache/internal/cache/repository_test.go +++ b/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 } } diff --git a/internal/clientcache/internal/cache/repository_token.go b/internal/clientcache/internal/cache/repository_token.go index 1e7f8f7b27..77823c16f9 100644 --- a/internal/clientcache/internal/cache/repository_token.go +++ b/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 } diff --git a/internal/clientcache/internal/daemon/server.go b/internal/clientcache/internal/daemon/server.go index 2c0037c34c..3f6b900b56 100644 --- a/internal/clientcache/internal/daemon/server.go +++ b/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 diff --git a/internal/clientcache/internal/daemon/token_handler_test.go b/internal/clientcache/internal/daemon/token_handler_test.go index b6bcbdaebe..d9a3960cd3 100644 --- a/internal/clientcache/internal/daemon/token_handler_test.go +++ b/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 } } diff --git a/internal/cmd/base/base.go b/internal/cmd/base/base.go index 2e2c0618d1..2e9cbc8c10 100644 --- a/internal/cmd/base/base.go +++ b/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) } } diff --git a/internal/cmd/base/keyring.go b/internal/cmd/base/keyring.go index c745f36246..3a2a5219ee 100644 --- a/internal/cmd/base/keyring.go +++ b/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,13 +112,16 @@ 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) @@ -123,10 +129,9 @@ func ReadTokenFromKeyring(ui cli.Ui, keyringType, tokenName string) *authtokens. 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) } diff --git a/internal/cmd/commands/config/token.go b/internal/cmd/commands/config/token.go index 01a28ab0cd..facab174d9 100644 --- a/internal/cmd/commands/config/token.go +++ b/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