db access denied error message improvement (#16228) (#17856)

* 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 <[email protected]> * 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 <[email protected]> Co-authored-by: Marek Smoliński <[email protected]>
gravitational · Oct 28, 2022 · 880367e · 880367e
1 parent 1599df9
commit 880367e
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 4 deletions.
diff --git 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
@@ -755,3 +755,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(),
+				WithExecer(&fakeExec{
+					commandPathBehavior: forceBasePath,
+					execOutput: map[string][]byte{
+						tt.wantBin: tt.stderr,
+					},
+				}),
+			}
+			c := NewCmdBuilder(tc, profile, database, "root", opts...)
+			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.options.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
@@ -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
@@ -2093,7 +2093,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
 		}