[v9] Improve error msg when client fails to auth in Teleport

api/client/client.go

@@ -153,10 +153,7 @@ func connect(ctx context.Context, cfg Config) (*Client, error) {
 	// sendError is used to send errors to errChan with context.
 	errChan := make(chan error)
 	sendError := func(err error) {
-		select {
-		case <-ctx.Done():
-		case errChan <- trace.Wrap(err):
-		}
+		errChan <- trace.Wrap(err)
 	}
 
 	// syncConnect is used to concurrently create multiple clients
@@ -248,7 +245,7 @@ func connect(ctx context.Context, cfg Config) (*Client, error) {
 	}()
 
 	var errs []error
-	for {
+	for errChan != nil {
 		select {
 		// Use the first client to successfully connect in syncConnect.
 		case clt := <-cltChan:
@@ -259,20 +256,23 @@ func connect(ctx context.Context, cfg Config) (*Client, error) {
 				errs = append(errs, trace.Wrap(err, ""))
 				continue
 			}
-			// errChan is closed, return errors.
-			if len(errs) == 0 {
-				if len(cfg.Addrs) == 0 && cfg.Dialer == nil {
-					// Some credentials don't require these fields. If no errors propagate, then they need to provide these fields.
-					return nil, trace.BadParameter("no connection methods found, try providing Dialer or Addrs in config")
-				}
-				// This case should never be reached with config validation and above case.
-				return nil, trace.Errorf("no connection methods found")
-			}
-			return nil, trace.Wrap(trace.NewAggregate(errs...), "all connection methods failed")
-		case <-ctx.Done():
-			return nil, trace.Wrap(ctx.Err())
+			errChan = nil
 		}
 	}
+
+	// errChan is closed, return errors.
+	if len(errs) == 0 {
+		if len(cfg.Addrs) == 0 && cfg.Dialer == nil {
+			// Some credentials don't require these fields. If no errors propagate, then they need to provide these fields.
+			return nil, trace.BadParameter("no connection methods found, try providing Dialer or Addrs in config")
+		}
+		// This case should never be reached with config validation and above case.
+		return nil, trace.Errorf("no connection methods found")
+	}
+	if ctx.Err() != nil {
+		errs = append(errs, trace.Wrap(ctx.Err()))
+	}
+	return nil, trace.Wrap(trace.NewAggregate(errs...), "all connection methods failed")
 }
 
 type (

api/client/client_test.go

@@ -21,7 +21,6 @@ import (
 	"crypto/tls"
 	"fmt"
 	"net"
-	"strings"
 	"testing"
 	"time"
 
@@ -338,7 +337,8 @@ func TestNew(t *testing.T) {
 			},
 		},
 		assertErr: func(t require.TestingT, err error, _ ...interface{}) {
-			require.True(t, strings.Contains(err.Error(), "all connection methods failed"))
+			require.Error(t, err)
+			require.Contains(t, err.Error(), "all connection methods failed")
 		},
 	}, {
 		desc: "fail to dial with no address or dialer.",
@@ -352,7 +352,8 @@ func TestNew(t *testing.T) {
 			},
 		},
 		assertErr: func(t require.TestingT, err error, _ ...interface{}) {
-			require.True(t, strings.Contains(err.Error(), "no connection methods found, try providing Dialer or Addrs in config"))
+			require.Error(t, err)
+			require.Contains(t, err.Error(), "no connection methods found, try providing Dialer or Addrs in config")
 		},
 	}}
 

integration/client_test.go

@@ -0,0 +1,76 @@
+/*
+Copyright 2020-2022 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package integration
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/google/uuid"
+	"github.com/gravitational/teleport/api/client"
+	"github.com/gravitational/teleport/lib/service"
+	"github.com/gravitational/teleport/lib/utils"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc"
+)
+
+// TestClientWithExpiredCredentialsAndDetailedErrorMessage creates and connects to the Auth service
+// using an expired user identity
+// We should receive an error message which contains the real cause (ssh: handshake)
+func TestClientWithExpiredCredentialsAndDetailedErrorMessage(t *testing.T) {
+	rc := NewInstance(InstanceConfig{
+		ClusterName: "root.example.com",
+		HostID:      uuid.New().String(),
+		NodeName:    Loopback,
+		log:         utils.NewLoggerForTests(),
+		Ports:       singleProxyPortSetup(),
+	})
+
+	rcConf := service.MakeDefaultConfig()
+	rcConf.DataDir = t.TempDir()
+	rcConf.Auth.Enabled = true
+	rcConf.Proxy.Enabled = true
+	rcConf.Proxy.DisableWebInterface = true
+	rcConf.SSH.Enabled = true
+	rcConf.Version = "v2"
+
+	username := mustGetCurrentUser(t).Username
+	rc.AddUser(username, []string{username})
+
+	err := rc.CreateEx(t, nil, rcConf)
+	require.NoError(t, err)
+	err = rc.Start()
+	require.NoError(t, err)
+	defer rc.StopAll()
+
+	// Create an expired identity file: ttl is 1 second in the past
+	identityFilePath := mustCreateUserIdentityFile(t, rc, username, -time.Second)
+
+	ctx, cancelFunc := context.WithTimeout(context.Background(), time.Second)
+	defer cancelFunc()
+	_, err = client.New(ctx, client.Config{
+		Addrs:       []string{rc.GetAuthAddr()},
+		Credentials: []client.Credentials{client.LoadIdentityFile(identityFilePath)},
+		DialOpts: []grpc.DialOption{
+			// ask for underlying errors
+			grpc.WithReturnConnectionError(),
+		},
+	})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "ssh: handshake failed")
+}

integration/proxy_helpers_test.go

@@ -511,13 +511,13 @@ func makeNodeConfig(nodeName, authAddr string) *service.Config {
 	return nodeConfig
 }
 
-func mustCreateUserIdentityFile(t *testing.T, tc *TeleInstance, username string) string {
+func mustCreateUserIdentityFile(t *testing.T, tc *TeleInstance, username string, ttl time.Duration) string {
 	key, err := libclient.NewKey()
 	require.NoError(t, err)
 	key.ClusterName = tc.Secrets.SiteName
 
 	sshCert, tlsCert, err := tc.Process.GetAuthServer().GenerateUserTestCerts(
-		key.Pub, username, time.Hour,
+		key.Pub, username, ttl,
 		constants.CertificateFormatStandard,
 		tc.Secrets.SiteName,
 	)

integration/proxy_test.go

@@ -641,7 +641,7 @@ func TestALPNProxyAuthClientConnectWithUserIdentity(t *testing.T) {
 	require.NoError(t, err)
 	defer rc.StopAll()
 
-	identityFilePath := mustCreateUserIdentityFile(t, rc, username)
+	identityFilePath := mustCreateUserIdentityFile(t, rc, username, time.Hour)
 
 	identity := client.LoadIdentityFile(identityFilePath)
 	require.NoError(t, err)