Skip to content

Commit

Permalink
db access denied error message improvement (#16228)
Browse files Browse the repository at this point in the history
* 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]>
  • Loading branch information
GavinFrazar and smallinsky authored Oct 26, 2022
1 parent 0f51099 commit ab91963
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 4 deletions.
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 @@ -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)
})
}
}
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 @@ -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
}
Expand Down

0 comments on commit ab91963

Please sign in to comment.