From c193cf51190e1d8f2f3a9828bb82c9c30cf33b91 Mon Sep 17 00:00:00 2001 From: Gavin Frazar Date: Thu, 27 Oct 2022 15:40:02 -0700 Subject: [PATCH] [v9] db access denied error message improvement (#17857) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * db access denied error message improvement (#16228) * Fix matchers not being printed in err message * Add helpful message when access is denied to db connect * Add test for converting command errors * Simplify error message when access to db is denied * Update lib/client/db/dbcmd/dbcmd_test.go Co-authored-by: Marek Smoliński * Remove unnecessary cmd.Run in test * Fix import order * Add full error to message * Fix formatting of github suggestion * Add troubleshooting link * Update troubleshooting link to use subheading Co-authored-by: Marek Smoliński * Fix test Co-authored-by: Marek Smoliński --- lib/client/db/dbcmd/dbcmd_test.go | 93 ++++++++++++++++++++++++++++++- lib/client/db/dbcmd/error.go | 7 +++ lib/services/role.go | 2 +- 3 files changed, 98 insertions(+), 4 deletions(-) diff --git a/lib/client/db/dbcmd/dbcmd_test.go b/lib/client/db/dbcmd/dbcmd_test.go index b9f80286f42af..e031ef2d3535d 100644 --- a/lib/client/db/dbcmd/dbcmd_test.go +++ b/lib/client/db/dbcmd/dbcmd_test.go @@ -21,15 +21,15 @@ import ( "path/filepath" "testing" + "github.com/gravitational/trace" + "github.com/stretchr/testify/require" + "github.com/gravitational/teleport/lib/client" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/fixtures" "github.com/gravitational/teleport/lib/observability/tracing" "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils" - - "github.com/gravitational/trace" - "github.com/stretchr/testify/require" ) type commandPathBehavior = int @@ -564,3 +564,90 @@ func TestGetConnectCommandNoAbsPathIsNoopWhenGivenRelativePath(t *testing.T) { require.NoError(t, err) require.Equal(t, "psql postgres://myUser@localhost:12345/mydb", got.String()) } + +func TestConvertCommandError(t *testing.T) { + t.Parallel() + homePath := t.TempDir() + conf := &client.Config{ + HomePath: homePath, + Host: "localhost", + WebProxyAddr: "localhost", + SiteName: "db.example.com", + Tracer: tracing.NoopProvider().Tracer("test"), + } + + tc, err := client.NewClient(conf) + require.NoError(t, err) + + profile := &client.ProfileStatus{ + Name: "example.com", + Username: "bob", + Dir: homePath, + } + + tests := []struct { + desc string + dbProtocol string + stderr []byte + wantBin string + wantStdErr string + }{ + { + desc: "converts access denied to helpful message", + dbProtocol: defaults.ProtocolMySQL, + stderr: []byte("access to db denied"), + wantBin: mysqlBin, + wantStdErr: "see your available logins, or ask your Teleport administrator", + }, + { + desc: "converts unrecognized redis error to helpful message", + dbProtocol: defaults.ProtocolRedis, + stderr: []byte("Unrecognized option or bad number of args for"), + wantBin: redisBin, + wantStdErr: "Please make sure 'redis-cli' with version 6.2 or newer is installed", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.desc, func(t *testing.T) { + t.Parallel() + + database := &tlsca.RouteToDatabase{ + Protocol: tt.dbProtocol, + Database: "DBName", + Username: "myUser", + ServiceName: "DBService", + } + + opts := []ConnectCommandFunc{ + WithLocalProxy("localhost", 12345, ""), + WithNoTLS(), + } + c := NewCmdBuilder(tc, profile, database, "root", opts...) + c.exe = &fakeExec{ + commandPathBehavior: forceBasePath, + execOutput: map[string][]byte{ + tt.wantBin: tt.stderr, + }, + } + c.uid = utils.NewFakeUID() + + cmd, err := c.GetConnectCommand() + require.NoError(t, err) + + // make sure the expected test bin is the command bin we got + require.Equal(t, cmd.Path, tt.wantBin) + + out, err := c.exe.RunCommand(cmd.Path) + require.NoError(t, err, "fakeExec.execOutput is not configured for bin %v", tt.wantBin) + + peakStderr := utils.NewCaptureNBytesWriter(PeakStderrSize) + _, peakErr := peakStderr.Write(out) + require.NoError(t, peakErr, "CaptureNBytesWriter should never return error") + + convertedErr := ConvertCommandError(cmd, nil, string(peakStderr.Bytes())) + require.ErrorContains(t, convertedErr, tt.wantStdErr) + }) + } +} diff --git a/lib/client/db/dbcmd/error.go b/lib/client/db/dbcmd/error.go index 1e9fafbcf109c..5c06fb99d6354 100644 --- a/lib/client/db/dbcmd/error.go +++ b/lib/client/db/dbcmd/error.go @@ -46,6 +46,13 @@ func ConvertCommandError(cmd *exec.Cmd, err error, peakStderr string) error { } } + lowerCaseStderr := strings.ToLower(peakStderr) + if strings.Contains(lowerCaseStderr, "access to db denied") { + fmtString := "%v: '%s' exited with the above error. Use 'tsh db ls' to see your available logins, " + + "or ask your Teleport administrator to grant you access." + + "\nSee https://goteleport.com/docs/database-access/troubleshooting/#access-to-db-denied for more information." + return trace.AccessDenied(fmtString, err, cmd.Path) + } return trace.Wrap(err) } diff --git a/lib/services/role.go b/lib/services/role.go index 653df7037a259..fcbf76e250f72 100644 --- a/lib/services/role.go +++ b/lib/services/role.go @@ -2043,7 +2043,7 @@ func (set RoleSet) CheckAccess(r AccessCheckable, mfa AccessMFAParams, matchers if !matchMatchers { if isDebugEnabled { errs = append(errs, fmt.Errorf("role=%v, match(matchers=%v)", - role.GetName(), err)) + role.GetName(), matchers)) } continue }