diff --git a/api/client/client.go b/api/client/client.go index 457d298752ac3..96042bdb339a5 100644 --- a/api/client/client.go +++ b/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 ( diff --git a/api/client/client_test.go b/api/client/client_test.go index b67aceb0c65c4..2b563aff3881a 100644 --- a/api/client/client_test.go +++ b/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") }, }} diff --git a/integration/client_test.go b/integration/client_test.go new file mode 100644 index 0000000000000..49b8475493367 --- /dev/null +++ b/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") +} diff --git a/integration/proxy_helpers_test.go b/integration/proxy_helpers_test.go index f2836ae7e2ae0..9a112de2e6609 100644 --- a/integration/proxy_helpers_test.go +++ b/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, ) diff --git a/integration/proxy_test.go b/integration/proxy_test.go index 833c95ad7ce5b..2593f66fe46f1 100644 --- a/integration/proxy_test.go +++ b/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)