From c3a9e6b74deb0c08274f27ecb1906f30d9b94084 Mon Sep 17 00:00:00 2001 From: Nic Klaassen Date: Fri, 17 Feb 2023 16:45:10 -0800 Subject: [PATCH] fix: improve tsh logs when skipping auto Access Request Currently the `tsh` debug log is polluted with "errors" created by the [automatic access request feature](https://goteleport.com/docs/access-controls/access-requests/resource-requests/?scope=enterprise#automatically-request-access-for-ssh) even in completely expected scenarios, e.g. when the user has no permission to create Resource Access Requests. Before this change: ``` $ tsh ssh -d alice@one-auth ...... 2023-02-17T15:30:16-08:00 DEBU [TSH] unable to request access to node error:[ ERROR REPORT: Original Error: *trace.BadParameterError user attempted a resource request but does not have any "search_as_roles" Stack Trace: github.com/gravitational/teleport/api@v0.0.0/client/client.go:880 github.com/gravitational/teleport/api/client.(*Client).CreateAccessRequest github.com/gravitational/teleport/tool/tsh/tsh.go:2896 main.accessRequestForSSH.func1 github.com/gravitational/teleport/lib/client/api.go:1351 github.com/gravitational/teleport/lib/client.(*TeleportClient).WithRootClusterClient github.com/gravitational/teleport/tool/tsh/tsh.go:2895 main.accessRequestForSSH github.com/gravitational/teleport/tool/tsh/tsh.go:2916 main.retryWithAccessRequest github.com/gravitational/teleport/tool/tsh/tsh.go:2993 main.onSSH github.com/gravitational/teleport/tool/tsh/tsh.go:1086 main.Run github.com/gravitational/teleport/tool/tsh/tsh.go:482 main.main runtime/proc.go:250 runtime.main runtime/asm_amd64.s:1598 runtime.goexit User Message: user attempted a resource request but does not have any "search_as_roles"] tsh/tsh.go:2920 ERROR REPORT: Original Error: *trace.AccessDeniedError access denied to alice connecting to one-auth:0@default@cluster-one Stack Trace: github.com/gravitational/teleport/lib/client/client.go:1633 github.com/gravitational/teleport/lib/client.NewNodeClient github.com/gravitational/teleport/lib/client/client.go:1563 github.com/gravitational/teleport/lib/client.(*ProxyClient).ConnectToNode github.com/gravitational/teleport/lib/client/api.go:1451 github.com/gravitational/teleport/lib/client.(*TeleportClient).ConnectToNode github.com/gravitational/teleport/lib/client/api.go:1525 github.com/gravitational/teleport/lib/client.(*TeleportClient).runShellOrCommandOnSingleNode github.com/gravitational/teleport/lib/client/api.go:1408 github.com/gravitational/teleport/lib/client.(*TeleportClient).SSH github.com/gravitational/teleport/tool/tsh/tsh.go:2995 main.onSSH.func1.1 github.com/gravitational/teleport/lib/client/api.go:504 github.com/gravitational/teleport/lib/client.RetryWithRelogin github.com/gravitational/teleport/tool/tsh/tsh.go:2994 main.onSSH.func1 github.com/gravitational/teleport/tool/tsh/tsh.go:2907 main.retryWithAccessRequest github.com/gravitational/teleport/tool/tsh/tsh.go:2993 main.onSSH github.com/gravitational/teleport/tool/tsh/tsh.go:1086 main.Run github.com/gravitational/teleport/tool/tsh/tsh.go:482 main.main runtime/proc.go:250 runtime.main runtime/asm_amd64.s:1598 runtime.goexit User Message: access denied to alice connecting to one-auth:0@default@cluster-one ``` After: ``` $ tsh ssh -d alice@one-auth ...... 2023-02-17T16:42:29-08:00 DEBU [TSH] Not attempting to automatically request access, reason: Resource Access Requests require usable "search_as_roles", none found for user "nklaassen" tsh/tsh.go:2922 ERROR REPORT: Original Error: *trace.AccessDeniedError access denied to alice connecting to one-auth:0@default@cluster-one Stack Trace: github.com/gravitational/teleport/lib/client/client.go:1633 github.com/gravitational/teleport/lib/client.NewNodeClient github.com/gravitational/teleport/lib/client/client.go:1563 github.com/gravitational/teleport/lib/client.(*ProxyClient).ConnectToNode github.com/gravitational/teleport/lib/client/api.go:1451 github.com/gravitational/teleport/lib/client.(*TeleportClient).ConnectToNode github.com/gravitational/teleport/lib/client/api.go:1525 github.com/gravitational/teleport/lib/client.(*TeleportClient).runShellOrCommandOnSingleNode github.com/gravitational/teleport/lib/client/api.go:1408 github.com/gravitational/teleport/lib/client.(*TeleportClient).SSH github.com/gravitational/teleport/tool/tsh/tsh.go:2997 main.onSSH.func1.1 github.com/gravitational/teleport/lib/client/api.go:504 github.com/gravitational/teleport/lib/client.RetryWithRelogin github.com/gravitational/teleport/tool/tsh/tsh.go:2996 main.onSSH.func1 github.com/gravitational/teleport/tool/tsh/tsh.go:2907 main.retryWithAccessRequest github.com/gravitational/teleport/tool/tsh/tsh.go:2995 main.onSSH github.com/gravitational/teleport/tool/tsh/tsh.go:1086 main.Run github.com/gravitational/teleport/tool/tsh/tsh.go:482 main.main runtime/proc.go:250 runtime.main runtime/asm_amd64.s:1598 runtime.goexit User Message: access denied to alice connecting to one-auth:0@default@cluster-one ``` --- lib/auth/access_request_test.go | 2 +- lib/services/access_request.go | 2 +- lib/services/access_request_test.go | 6 +++--- tool/tsh/tsh.go | 8 +++++--- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/auth/access_request_test.go b/lib/auth/access_request_test.go index 280718f9057c6..e2cb9f448b339 100644 --- a/lib/auth/access_request_test.go +++ b/lib/auth/access_request_test.go @@ -261,7 +261,7 @@ func testSingleAccessRequests(t *testing.T, testPack *accessRequestTestPack) { desc: "no search_as_roles", requester: "nobody", requestResources: []string{"prod"}, - expectRequestError: trace.BadParameter(`user attempted a resource request but does not have any "search_as_roles"`), + expectRequestError: trace.AccessDenied(`Resource Access Requests require usable "search_as_roles", none found for user "nobody"`), }, } for _, tc := range testCases { diff --git a/lib/services/access_request.go b/lib/services/access_request.go index cd17ce49acae3..2c857d03302c6 100644 --- a/lib/services/access_request.go +++ b/lib/services/access_request.go @@ -184,7 +184,7 @@ func (m *RequestValidator) applicableSearchAsRoles(ctx context.Context, resource rolesToRequest = append(rolesToRequest, roleName) } if len(rolesToRequest) == 0 { - return nil, trace.BadParameter(`user attempted a resource request but does not have any "search_as_roles"`) + return nil, trace.AccessDenied(`Resource Access Requests require usable "search_as_roles", none found for user %q`, m.user.GetName()) } // Prune the list of roles to request to only those which may be necessary diff --git a/lib/services/access_request_test.go b/lib/services/access_request_test.go index 6f1baea7b9b48..8d9c2d8b4c792 100644 --- a/lib/services/access_request_test.go +++ b/lib/services/access_request_test.go @@ -974,13 +974,13 @@ func TestRolesForResourceRequest(t *testing.T) { desc: "deny search", currentRoles: []string{"db-response-team", "deny-db-search"}, requestResourceIDs: resourceIDs, - expectError: trace.BadParameter(`user attempted a resource request but does not have any "search_as_roles"`), + expectError: trace.AccessDenied(`Resource Access Requests require usable "search_as_roles", none found for user "test-user"`), }, { desc: "deny request", currentRoles: []string{"db-response-team", "deny-db-request"}, requestResourceIDs: resourceIDs, - expectError: trace.BadParameter(`user attempted a resource request but does not have any "search_as_roles"`), + expectError: trace.AccessDenied(`Resource Access Requests require usable "search_as_roles", none found for user "test-user"`), }, { desc: "multi allowed roles", @@ -1012,7 +1012,7 @@ func TestRolesForResourceRequest(t *testing.T) { desc: "no allowed roles", currentRoles: nil, requestResourceIDs: resourceIDs, - expectError: trace.BadParameter(`user attempted a resource request but does not have any "search_as_roles"`), + expectError: trace.AccessDenied(`Resource Access Requests require usable "search_as_roles", none found for user "test-user"`), }, } for _, tc := range testCases { diff --git a/tool/tsh/tsh.go b/tool/tsh/tsh.go index 097e60bdfd6c0..45c7edfaf4772 100644 --- a/tool/tsh/tsh.go +++ b/tool/tsh/tsh.go @@ -2775,9 +2775,11 @@ func retryWithAccessRequest(cf *CLIConf, tc *client.TeleportClient, fn func() er // Try to construct an access request for this node. req, err := accessRequestForSSH(cf.Context, tc) if err != nil { - // We can't request access to the node or it doesn't exist, return the - // original error but put this one in the debug log. - log.WithError(err).Debug("unable to request access to node") + // We can't request access to the node or we couldn't query the ID. Log + // a short debug message in case this is unexpected, but return the + // original AccessDenied error from the ssh attempt which is likely to + // be far more relevant to the user. + log.Debugf("Not attempting to automatically request access, reason: %v", err) return trace.Wrap(origErr) } cf.RequestID = req.GetName()