Check if resource request is possible before attempting (#13586) (#13620

)

api/types/access_request.go

@@ -98,6 +98,11 @@ type AccessRequest interface {
 	GetLoginHint() string
 	// SetLoginHint sets the requested login hint.
 	SetLoginHint(string)
+	// GetDryRun returns true if this request should not be created and is only
+	// a dry run to validate request capabilities.
+	GetDryRun() bool
+	// SetDryRun sets the dry run flag on the request.
+	SetDryRun(bool)
 }
 
 // NewAccessRequest assembles an AccessRequest resource.
@@ -396,6 +401,17 @@ func (r *AccessRequestV3) SetLoginHint(login string) {
 	r.Spec.LoginHint = login
 }
 
+// GetDryRun returns true if this request should not be created and is only
+// a dry run to validate request capabilities.
+func (r *AccessRequestV3) GetDryRun() bool {
+	return r.Spec.DryRun
+}
+
+// SetDryRun sets the dry run flag on the request.
+func (r *AccessRequestV3) SetDryRun(dryRun bool) {
+	r.Spec.DryRun = dryRun
+}
+
 // String returns a text representation of this AccessRequest
 func (r *AccessRequestV3) String() string {
 	return fmt.Sprintf("AccessRequest(user=%v,roles=%+v)", r.Spec.User, r.Spec.Roles)

api/types/types.proto

@@ -1460,6 +1460,10 @@ message AccessRequestSpecV3 {
     // LoginHint is used as a hint for search-based access requests to select
     // roles based on the login the user is attempting.
     string LoginHint = 15 [ (gogoproto.jsontag) = "login_hint,omitempty" ];
+
+    // DryRun indicates that the request should not actually be created, the
+    // auth server should only validate the access request.
+    bool DryRun = 16 [ (gogoproto.jsontag) = "dry_run,omitempty" ];
 }
 
 // AccessRequestFilter encodes filter params for access requests.

lib/auth/access_request_test.go

@@ -212,7 +212,7 @@ func TestAccessRequest(t *testing.T) {
 			desc:               "no search_as_roles",
 			requester:          "nobody",
 			requestResources:   []string{"prod"},
-			expectRequestError: trace.AccessDenied(`user does not have any "search_as_roles" which are valid for this request`),
+			expectRequestError: trace.BadParameter(`user attempted a resource request but does not have any "search_as_roles"`),
 		},
 	}
 	for _, tc := range testCases {

lib/auth/auth.go

@@ -2558,6 +2558,13 @@ func (a *Server) CreateAccessRequest(ctx context.Context, req types.AccessReques
 			req.SetExpiry(pexp)
 		}
 	}
+
+	if req.GetDryRun() {
+		// Made it this far with no errors, return before creating the request
+		// if this is a dry run.
+		return nil
+	}
+
 	if err := a.DynamicAccessExt.CreateAccessRequest(ctx, req); err != nil {
 		return trace.Wrap(err)
 	}

lib/services/access_request.go

@@ -1081,7 +1081,7 @@ func (m *RequestValidator) setRolesForResourceRequest(ctx context.Context, req t
 		rolesToRequest = append(rolesToRequest, roleName)
 	}
 	if len(rolesToRequest) == 0 {
-		return trace.AccessDenied(`user does not have any "search_as_roles" which are valid for this request`)
+		return trace.BadParameter(`user attempted a resource request but does not have any "search_as_roles"`)
 	}
 	req.SetRoles(rolesToRequest)
 	return nil

lib/services/access_request_test.go

@@ -967,13 +967,13 @@ func TestRolesForResourceRequest(t *testing.T) {
 			desc:               "deny search",
 			currentRoles:       []string{"db-response-team", "deny-db-search"},
 			requestResourceIDs: resourceIDs,
-			expectError:        trace.AccessDenied(`user does not have any "search_as_roles" which are valid for this request`),
+			expectError:        trace.BadParameter(`user attempted a resource request but does not have any "search_as_roles"`),
 		},
 		{
 			desc:               "deny request",
 			currentRoles:       []string{"db-response-team", "deny-db-request"},
 			requestResourceIDs: resourceIDs,
-			expectError:        trace.AccessDenied(`user does not have any "search_as_roles" which are valid for this request`),
+			expectError:        trace.BadParameter(`user attempted a resource request but does not have any "search_as_roles"`),
 		},
 		{
 			desc:                 "multi allowed roles",
@@ -1005,7 +1005,7 @@ func TestRolesForResourceRequest(t *testing.T) {
 			desc:               "no allowed roles",
 			currentRoles:       nil,
 			requestResourceIDs: resourceIDs,
-			expectError:        trace.AccessDenied(`user does not have any "search_as_roles" which are valid for this request`),
+			expectError:        trace.BadParameter(`user attempted a resource request but does not have any "search_as_roles"`),
 		},
 	}
 	for _, tc := range testCases {

lib/services/local/dynamic_access.go

@@ -46,6 +46,9 @@ func (s *DynamicAccessService) CreateAccessRequest(ctx context.Context, req type
 	if err := services.ValidateAccessRequest(req); err != nil {
 		return trace.Wrap(err)
 	}
+	if req.GetDryRun() {
+		return trace.BadParameter("dry run access request made it to DynamicAccessService, this is a bug")
+	}
 	item, err := itemFromAccessRequest(req)
 	if err != nil {
 		return trace.Wrap(err)

tool/tsh/tsh.go

@@ -2384,6 +2384,20 @@ func accessRequestForSSH(ctx context.Context, tc *client.TeleportClient) (types.
 		return nil, trace.Wrap(err)
 	}
 	req.SetLoginHint(tc.HostLogin)
+
+	// Set the DryRun flag and send the request to auth for full validation. If
+	// the user has no search_as_roles or is not allowed to SSH to the host with
+	// the requested login, we will get an error here.
+	req.SetDryRun(true)
+	req.SetRequestReason("Dry run, this request will not be created. If you see this, there is a bug.")
+	if err := tc.WithRootClusterClient(ctx, func(clt auth.ClientI) error {
+		return trace.Wrap(clt.CreateAccessRequest(ctx, req))
+	}); err != nil {
+		return nil, trace.Wrap(err)
+	}
+	req.SetDryRun(false)
+	req.SetRequestReason("")
+
 	return req, nil
 }
 
@@ -2398,16 +2412,12 @@ 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 trace.IsAccessDenied(err) || trace.IsNotFound(err) {
+	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")
 		return trace.Wrap(origErr)
 	}
-	if err != nil {
-		// Unexpected error, return it.
-		return trace.Wrap(err)
-	}
 	cf.RequestID = req.GetName()
 
 	// Print and log the original AccessDenied error.
@@ -2435,8 +2445,7 @@ func retryWithAccessRequest(cf *CLIConf, tc *client.TeleportClient, fn func() er
 	fmt.Fprint(os.Stdout, "Creating request...\n")
 	// Always create access request against the root cluster.
 	if err := tc.WithRootClusterClient(cf.Context, func(clt auth.ClientI) error {
-		err := clt.CreateAccessRequest(cf.Context, req)
-		return trace.Wrap(err)
+		return trace.Wrap(clt.CreateAccessRequest(cf.Context, req))
 	}); err != nil {
 		return trace.Wrap(err)
 	}

tool/tsh/tsh_test.go

@@ -615,15 +615,26 @@ func TestSSHAccessRequest(t *testing.T) {
 	}))
 	require.NoError(t, err)
 
-	// won't request access unless possible
+	// won't request if can't list node
 	err = Run(ctx, []string{
 		"ssh",
 		"--insecure",
+		"--request-reason", "reason here to bypass prompt",
 		fmt.Sprintf("%s@%s", user.Username, sshHostnameNoAccess),
 		"echo", "test",
 	}, setHomePath(tmpHomePath))
 	require.Error(t, err)
 
+	// won't request if can't login with username
+	err = Run(ctx, []string{
+		"ssh",
+		"--insecure",
+		"--request-reason", "reason here to bypass prompt",
+		fmt.Sprintf("%s@%s", "not-a-username", sshHostname),
+		"echo", "test",
+	}, setHomePath(tmpHomePath))
+	require.Error(t, err)
+
 	// won't request to non-existent node
 	err = Run(ctx, []string{
 		"ssh",