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

[v9] db access denied error message improvement #17857

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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,
},
}
Comment on lines +628 to +633
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to update this test to use c.exe instead of c.options.exe since exe was moved after v9 and not backported, but it does not affect this PR.

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)
})
}
}
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") {
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)
}

Expand Down
2 changes: 1 addition & 1 deletion lib/services/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down