Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

db access denied error message improvement #16228

Merged
Merged
93 changes: 90 additions & 3 deletions lib/client/db/dbcmd/dbcmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -590,3 +590,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)
})
}
}
7 changes: 7 additions & 0 deletions lib/client/db/dbcmd/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
GavinFrazar marked this conversation as resolved.
Show resolved Hide resolved
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 for more information."
return trace.AccessDenied(fmtString, err, cmd.Path)
}
return trace.Wrap(err)
}

Expand Down
2 changes: 1 addition & 1 deletion lib/services/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -2018,7 +2018,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
}
Expand Down
8 changes: 4 additions & 4 deletions tool/tsh/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ import (
"strings"
"text/template"

"github.com/ghodss/yaml"
"github.com/gravitational/trace"
"golang.org/x/sync/errgroup"

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/types"
Expand All @@ -39,10 +43,6 @@ import (
"github.com/gravitational/teleport/lib/srv/alpnproxy/common"
"github.com/gravitational/teleport/lib/tlsca"
"github.com/gravitational/teleport/lib/utils"

"github.com/ghodss/yaml"
"github.com/gravitational/trace"
"golang.org/x/sync/errgroup"
)

// onListDatabases implements "tsh db ls" command.
Expand Down