[v10] Fix resource access requests for apps

lib/services/access_request.go

@@ -18,11 +18,14 @@ package services
 
 import (
 	"context"
+	"fmt"
 	"sort"
+	"strings"
 	"time"
 
 	"github.com/google/go-cmp/cmp"
 
+	"github.com/gravitational/teleport/api/client/proto"
 	apidefaults "github.com/gravitational/teleport/api/defaults"
 	"github.com/gravitational/teleport/api/types"
 	apiutils "github.com/gravitational/teleport/api/utils"
@@ -1301,11 +1304,7 @@ func MarshalAccessRequest(accessRequest types.AccessRequest, opts ...MarshalOpti
 // PruneResourceRequestGetter is the access interface necessary for PruneResourceRequestRoles.
 type PruneResourceRequestGetter interface {
 	GetRole(ctx context.Context, name string) (types.Role, error)
-	GetNode(ctx context.Context, namespace string, name string) (types.Server, error)
-	GetKubeServices(ctx context.Context) ([]types.Server, error)
-	GetDatabase(ctx context.Context, name string) (types.Database, error)
-	GetApp(ctx context.Context, name string) (types.Application, error)
-	GetWindowsDesktops(ctx context.Context, filter types.WindowsDesktopFilter) ([]types.WindowsDesktop, error)
+	ListResources(ctx context.Context, req proto.ListResourcesRequest) (*types.ListResourcesResponse, error)
 }
 
 // PruneResourceRequestRoles takes an access request and does one of two things:
@@ -1450,55 +1449,85 @@ func roleAllowsResource(
 }
 
 func getResources(ctx context.Context, getter PruneResourceRequestGetter, resourceIDs []types.ResourceID) ([]types.ResourceWithLabels, error) {
-	var resources []types.ResourceWithLabels
+	resourceNamesByKind := make(map[string][]string)
 	for _, resourceID := range resourceIDs {
-		switch resourceID.Kind {
-		case types.KindNode:
-			node, err := getter.GetNode(ctx, apidefaults.Namespace, resourceID.Name)
-			if err != nil {
-				return nil, trace.Wrap(err)
-			}
-			resources = append(resources, node)
-		case types.KindKubernetesCluster:
-			kubeServices, err := getter.GetKubeServices(ctx)
+		resourceNamesByKind[resourceID.Kind] = append(resourceNamesByKind[resourceID.Kind], resourceID.Name)
+	}
+	var resources []types.ResourceWithLabels
+	for kind, resourceNames := range resourceNamesByKind {
+		req := proto.ListResourcesRequest{
+			ResourceType:        MapResourceKindToListResourcesType(kind),
+			PredicateExpression: anyNameMatcher(resourceNames),
+			Limit:               int32(len(resourceNames)),
+		}
+		resp, err := getter.ListResources(ctx, req)
+		if err != nil {
+			return nil, trace.Wrap(err)
+		}
+		for _, result := range resp.Resources {
+			leafResources, err := MapListResourcesResultToLeafResource(result, kind)
 			if err != nil {
 				return nil, trace.Wrap(err)
 			}
-			for _, kubeService := range kubeServices {
-				for _, kubeCluster := range kubeService.GetKubernetesClusters() {
-					if kubeCluster.Name != resourceID.Name {
-						continue
-					}
-					kubeV3, err := types.NewKubernetesClusterV3FromLegacyCluster(kubeService.GetNamespace(), kubeCluster)
-					if err != nil {
-						return nil, trace.Wrap(err)
-					}
-					resources = append(resources, kubeV3)
+			resources = append(resources, leafResources...)
+		}
+	}
+	return resources, nil
+}
+
+// anyNameMatcher returns a PredicateExpression which matches any of a given list
+// of names. Given names will be escaped and quoted when building the expression.
+func anyNameMatcher(names []string) string {
+	matchers := make([]string, len(names))
+	for i := range names {
+		matchers[i] = fmt.Sprintf(`name == %q`, names[i])
+	}
+	return strings.Join(matchers, " || ")
+}
+
+// MapResourceKindToListResourcesType returns the value to use for ResourceType in a
+// ListResourcesRequest based on the kind of resource you're searching for.
+// Necessary because some resource kinds don't support ListResources directly,
+// so you have to list the parent kind. Use MapListResourcesResultToLeafResource to map back
+// to the given kind.
+func MapResourceKindToListResourcesType(kind string) string {
+	switch kind {
+	case types.KindApp:
+		return types.KindAppServer
+	case types.KindDatabase:
+		return types.KindDatabaseServer
+	case types.KindKubernetesCluster:
+		return types.KindKubeService
+	default:
+		return kind
+	}
+}
+
+// MapListResourcesResultToLeafResource is the inverse of
+// MapResourceKindToListResourcesType, after the ListResources call it maps the
+// result back to the kind we really want. `hint` should be the name of the
+// desired resource kind, used to disambiguate normal SSH nodes and kubernetes
+// services which are both returned as `types.Server`.
+func MapListResourcesResultToLeafResource(resource types.ResourceWithLabels, hint string) (types.ResourcesWithLabels, error) {
+	switch r := resource.(type) {
+	case types.AppServer:
+		return types.ResourcesWithLabels{r.GetApp()}, nil
+	case types.DatabaseServer:
+		return types.ResourcesWithLabels{r.GetDatabase()}, nil
+	case types.Server:
+		if hint == types.KindKubernetesCluster {
+			kubeClusters := r.GetKubernetesClusters()
+			resources := make(types.ResourcesWithLabels, len(kubeClusters))
+			for i := range kubeClusters {
+				resource, err := types.NewKubernetesClusterV3FromLegacyCluster(apidefaults.Namespace, kubeClusters[i])
+				if err != nil {
+					return nil, trace.Wrap(err)
 				}
+				resources[i] = resource
 			}
-		case types.KindDatabase:
-			db, err := getter.GetDatabase(ctx, resourceID.Name)
-			if err != nil {
-				return nil, trace.Wrap(err)
-			}
-			resources = append(resources, db)
-		case types.KindApp:
-			app, err := getter.GetApp(ctx, resourceID.Name)
-			if err != nil {
-				return nil, trace.Wrap(err)
-			}
-			resources = append(resources, app)
-		case types.KindWindowsDesktop:
-			desktops, err := getter.GetWindowsDesktops(ctx, types.WindowsDesktopFilter{
-				Name: resourceID.Name,
-			})
-			if err != nil {
-				return nil, trace.Wrap(err)
-			}
-			for _, desktop := range desktops {
-				resources = append(resources, desktop)
-			}
+			return resources, nil
 		}
+	default:
 	}
-	return resources, nil
+	return types.ResourcesWithLabels{resource}, nil
 }

lib/services/access_request_test.go

@@ -18,8 +18,10 @@ package services
 
 import (
 	"context"
+	"strings"
 	"testing"
 
+	"github.com/gravitational/teleport/api/client/proto"
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/lib/fixtures"
 
@@ -31,13 +33,13 @@ import (
 
 // mockGetter mocks the UserAndRoleGetter interface.
 type mockGetter struct {
-	users        map[string]types.User
-	roles        map[string]types.Role
-	nodes        map[string]types.Server
-	kubeServices []types.Server
-	dbs          map[string]types.Database
-	apps         map[string]types.Application
-	desktops     map[string]types.WindowsDesktop
+	users       map[string]types.User
+	roles       map[string]types.Role
+	nodes       map[string]types.Server
+	kubeServers map[string]types.Server
+	dbServers   map[string]types.DatabaseServer
+	appServers  map[string]types.AppServer
+	desktops    map[string]types.WindowsDesktop
 }
 
 // user inserts a new user with the specified roles and returns the username.
@@ -78,40 +80,37 @@ func (m *mockGetter) GetRoles(ctx context.Context) ([]types.Role, error) {
 	return roles, nil
 }
 
-func (m *mockGetter) GetNode(ctx context.Context, namespace string, name string) (types.Server, error) {
-	node, ok := m.nodes[name]
-	if !ok {
-		return nil, trace.NotFound("no such node: %q", name)
+// ListResources is a very dumb implementation for the mockGetter that just
+// returns all resources which have names matching the request
+// PredicateExpression.
+func (m *mockGetter) ListResources(ctx context.Context, req proto.ListResourcesRequest) (*types.ListResourcesResponse, error) {
+	resp := &types.ListResourcesResponse{}
+	for nodeName, node := range m.nodes {
+		if strings.Contains(req.PredicateExpression, nodeName) {
+			resp.Resources = append(resp.Resources, types.ResourceWithLabels(node))
+		}
 	}
-	return node, nil
-}
-
-func (m *mockGetter) GetKubeServices(ctx context.Context) ([]types.Server, error) {
-	return append([]types.Server{}, m.kubeServices...), nil
-}
-
-func (m *mockGetter) GetDatabase(ctx context.Context, name string) (types.Database, error) {
-	db, ok := m.dbs[name]
-	if !ok {
-		return nil, trace.NotFound("no such db: %q", name)
+	for kubeName, kubeService := range m.kubeServers {
+		if strings.Contains(req.PredicateExpression, kubeName) {
+			resp.Resources = append(resp.Resources, types.ResourceWithLabels(kubeService))
+		}
 	}
-	return db, nil
-}
-
-func (m *mockGetter) GetApp(ctx context.Context, name string) (types.Application, error) {
-	app, ok := m.apps[name]
-	if !ok {
-		return nil, trace.NotFound("no such app: %q", name)
+	for dbName, dbServer := range m.dbServers {
+		if strings.Contains(req.PredicateExpression, dbName) {
+			resp.Resources = append(resp.Resources, dbServer)
+		}
 	}
-	return app, nil
-}
-
-func (m *mockGetter) GetWindowsDesktops(ctx context.Context, filter types.WindowsDesktopFilter) ([]types.WindowsDesktop, error) {
-	desktop, ok := m.desktops[filter.Name]
-	if !ok {
-		return nil, trace.NotFound("no such desktop: %q", filter.Name)
+	for appName, appServer := range m.appServers {
+		if strings.Contains(req.PredicateExpression, appName) {
+			resp.Resources = append(resp.Resources, appServer)
+		}
 	}
-	return []types.WindowsDesktop{desktop}, nil
+	for desktopName, desktop := range m.desktops {
+		if strings.Contains(req.PredicateExpression, desktopName) {
+			resp.Resources = append(resp.Resources, desktop)
+		}
+	}
+	return resp, nil
 }
 
 // TestReviewThresholds tests various review threshold scenarios
@@ -1044,12 +1043,13 @@ func TestPruneRequestRoles(t *testing.T) {
 	ctx := context.Background()
 
 	g := &mockGetter{
-		roles:    make(map[string]types.Role),
-		users:    make(map[string]types.User),
-		nodes:    make(map[string]types.Server),
-		dbs:      make(map[string]types.Database),
-		apps:     make(map[string]types.Application),
-		desktops: make(map[string]types.WindowsDesktop),
+		roles:       make(map[string]types.Role),
+		users:       make(map[string]types.User),
+		nodes:       make(map[string]types.Server),
+		kubeServers: make(map[string]types.Server),
+		dbServers:   make(map[string]types.DatabaseServer),
+		appServers:  make(map[string]types.AppServer),
+		desktops:    make(map[string]types.WindowsDesktop),
 	}
 
 	// set up test roles
@@ -1142,6 +1142,12 @@ func TestPruneRequestRoles(t *testing.T) {
 				"owner": "node-admins",
 			},
 		},
+		{
+			name: "admins-node-2",
+			labels: map[string]string{
+				"owner": "node-admins",
+			},
+		},
 		{
 			name: "denied-node",
 		},
@@ -1161,7 +1167,7 @@ func TestPruneRequestRoles(t *testing.T) {
 		},
 	}, nil)
 	require.NoError(t, err)
-	g.kubeServices = append(g.kubeServices, kube)
+	g.kubeServers[kube.GetName()] = kube
 
 	db, err := types.NewDatabaseV3(types.Metadata{
 		Name: "db",
@@ -1170,15 +1176,25 @@ func TestPruneRequestRoles(t *testing.T) {
 		URI:      "example.com:3000",
 	})
 	require.NoError(t, err)
-	g.dbs[db.GetName()] = db
+	dbServer, err := types.NewDatabaseServerV3(types.Metadata{
+		Name: db.GetName(),
+	}, types.DatabaseServerSpecV3{
+		HostID:   "db-server",
+		Hostname: "db-server",
+		Database: db,
+	})
+	require.NoError(t, err)
+	g.dbServers[dbServer.GetName()] = dbServer
 
 	app, err := types.NewAppV3(types.Metadata{
 		Name: "app",
 	}, types.AppSpecV3{
 		URI: "example.com:3000",
 	})
 	require.NoError(t, err)
-	g.apps[app.GetName()] = app
+	appServer, err := types.NewAppServerV3FromApp(app, "app-server", "app-server")
+	require.NoError(t, err)
+	g.appServers[app.GetName()] = appServer
 
 	desktop, err := types.NewWindowsDesktopV3("windows", nil, types.WindowsDesktopSpecV3{
 		Addr: "example.com:3001",
@@ -1222,6 +1238,24 @@ func TestPruneRequestRoles(t *testing.T) {
 			// With "responder" login hint, only request node-access.
 			expectRoles: []string{"node-access"},
 		},
+		{
+			desc: "multiple nodes",
+			requestResourceIDs: []types.ResourceID{
+				{
+					ClusterName: clusterName,
+					Kind:        types.KindNode,
+					Name:        "admins-node",
+				},
+				{
+					ClusterName: clusterName,
+					Kind:        types.KindNode,
+					Name:        "admins-node-2",
+				},
+			},
+			loginHint: "responder",
+			// With "responder" login hint, only request node-access.
+			expectRoles: []string{"node-access"},
+		},
 		{
 			desc: "root login hint",
 			requestResourceIDs: []types.ResourceID{