Show resources in Slack notification for access requests (#32887)

This change updates Slack notifications for resource-based access
requests to include the resources being requested.

api/accessrequest/access_request.go

@@ -0,0 +1,181 @@
+/*
+Copyright 2023 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package accessrequest
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/gravitational/trace"
+
+	"github.com/gravitational/teleport/api/client"
+	"github.com/gravitational/teleport/api/client/proto"
+	"github.com/gravitational/teleport/api/types"
+)
+
+type ListResourcesRequestOption func(*proto.ListResourcesRequest)
+
+// GetResourcesByKind is an alternative to client.GetResourcesWithFilters
+// that searches with the resource kinds used in access requests instead of the
+// resource types expected by ListResources.
+//
+// The ResourceType field in the request should not be set by the caller, as
+// it will be overridden.
+func GetResourcesByKind(ctx context.Context, clt client.ListResourcesClient, req proto.ListResourcesRequest, kind string) ([]types.ResourceWithLabels, error) {
+	req.ResourceType = mapResourceKindToListResourcesType(kind)
+	results, err := client.GetResourcesWithFilters(ctx, clt, req)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	resources := make([]types.ResourceWithLabels, 0, len(results))
+	for _, result := range results {
+		leafResource, err := mapListResourcesResultToLeafResource(result, kind)
+		if err != nil {
+			return nil, trace.Wrap(err)
+		}
+		resources = append(resources, leafResource)
+	}
+	return resources, nil
+}
+
+// GetResourceDetails gets extra details for a list of resources in a given cluster.
+func GetResourceDetails(ctx context.Context, clusterName string, lister client.ListResourcesClient, ids []types.ResourceID) (map[string]types.ResourceDetails, error) {
+	var resourceIDs []types.ResourceID
+	for _, resourceID := range ids {
+		// We're interested in hostname or friendly name details. These apply to
+		// nodes, app servers, and user groups.
+		switch resourceID.Kind {
+		case types.KindNode, types.KindApp, types.KindUserGroup:
+			resourceIDs = append(resourceIDs, resourceID)
+		}
+	}
+
+	withExtraRoles := func(req *proto.ListResourcesRequest) {
+		req.UseSearchAsRoles = true
+		req.UsePreviewAsRoles = true
+	}
+
+	resources, err := GetResourcesByResourceIDs(ctx, lister, resourceIDs, withExtraRoles)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	result := make(map[string]types.ResourceDetails)
+	for _, resource := range resources {
+		friendlyName := types.FriendlyName(resource)
+
+		// No friendly name was found, so skip to the next resource.
+		if friendlyName == "" {
+			continue
+		}
+
+		id := types.ResourceID{
+			ClusterName: clusterName,
+			Kind:        resource.GetKind(),
+			Name:        resource.GetName(),
+		}
+		result[types.ResourceIDToString(id)] = types.ResourceDetails{
+			FriendlyName: friendlyName,
+		}
+	}
+
+	return result, nil
+}
+
+// GetResourceIDsByCluster will return resource IDs grouped by cluster.
+func GetResourceIDsByCluster(r types.AccessRequest) map[string][]types.ResourceID {
+	resourceIDsByCluster := make(map[string][]types.ResourceID)
+	for _, resourceID := range r.GetRequestedResourceIDs() {
+		resourceIDsByCluster[resourceID.ClusterName] = append(resourceIDsByCluster[resourceID.ClusterName], resourceID)
+	}
+	return resourceIDsByCluster
+}
+
+// GetResourcesByResourceID gets a list of resources by their resource IDs.
+func GetResourcesByResourceIDs(ctx context.Context, lister client.ListResourcesClient, resourceIDs []types.ResourceID, opts ...ListResourcesRequestOption) ([]types.ResourceWithLabels, error) {
+	resourceNamesByKind := make(map[string][]string)
+	for _, resourceID := range resourceIDs {
+		resourceNamesByKind[resourceID.Kind] = append(resourceNamesByKind[resourceID.Kind], resourceID.Name)
+	}
+	var resources []types.ResourceWithLabels
+	for kind, resourceNames := range resourceNamesByKind {
+		req := proto.ListResourcesRequest{
+			PredicateExpression: anyNameMatcher(resourceNames),
+			Limit:               int32(len(resourceNames)),
+		}
+		for _, opt := range opts {
+			opt(&req)
+		}
+		resp, err := GetResourcesByKind(ctx, lister, req, kind)
+		if err != nil {
+			return nil, trace.Wrap(err)
+		}
+		resources = append(resources, resp...)
+	}
+	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(`resource.metadata.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.KindKubeServer
+	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.ResourceWithLabels, error) {
+	switch r := resource.(type) {
+	case types.AppServer:
+		return r.GetApp(), nil
+	case types.KubeServer:
+		return r.GetCluster(), nil
+	case types.DatabaseServer:
+		return r.GetDatabase(), nil
+	case types.Server:
+		if hint == types.KindKubernetesCluster {
+			return nil, trace.BadParameter("expected kubernetes server, got server")
+		}
+	default:
+	}
+	return resource, nil
+}

api/accessrequest/access_request_test.go

@@ -0,0 +1,127 @@
+/*
+Copyright 2023 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package accessrequest
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/gravitational/teleport/api/client/proto"
+	"github.com/gravitational/teleport/api/types"
+)
+
+func newNode(t *testing.T, name, hostname string) types.Server {
+	t.Helper()
+	node, err := types.NewServer(name, types.KindNode,
+		types.ServerSpecV2{
+			Hostname: hostname,
+		})
+	require.NoError(t, err)
+	return node
+}
+
+func newApp(t *testing.T, name, description, origin string) types.Application {
+	t.Helper()
+	app, err := types.NewAppV3(types.Metadata{
+		Name:        name,
+		Description: description,
+		Labels: map[string]string{
+			types.OriginLabel: origin,
+		},
+	},
+		types.AppSpecV3{
+			URI:        "https://some-addr.com",
+			PublicAddr: "https://some-addr.com",
+		})
+	require.NoError(t, err)
+	return app
+}
+
+func newUserGroup(t *testing.T, name, description, origin string) types.UserGroup {
+	t.Helper()
+	userGroup, err := types.NewUserGroup(types.Metadata{
+		Name:        name,
+		Description: description,
+		Labels: map[string]string{
+			types.OriginLabel: origin,
+		},
+	}, types.UserGroupSpecV1{})
+	require.NoError(t, err)
+	return userGroup
+}
+
+func newResourceID(clusterName, kind, name string) types.ResourceID {
+	return types.ResourceID{
+		ClusterName: clusterName,
+		Kind:        kind,
+		Name:        name,
+	}
+}
+
+type mockResourceLister struct {
+	resources []types.ResourceWithLabels
+}
+
+func (m *mockResourceLister) ListResources(ctx context.Context, _ proto.ListResourcesRequest) (*types.ListResourcesResponse, error) {
+	return &types.ListResourcesResponse{
+		Resources: m.resources,
+	}, nil
+}
+
+func TestGetResourceDetails(t *testing.T) {
+	clusterName := "cluster"
+
+	presence := &mockResourceLister{
+		resources: []types.ResourceWithLabels{
+			newNode(t, "node1", "hostname 1"),
+			newApp(t, "app1", "friendly app 1", types.OriginDynamic),
+			newApp(t, "app2", "friendly app 2", types.OriginDynamic),
+			newApp(t, "app3", "friendly app 3", types.OriginOkta),
+			newUserGroup(t, "group1", "friendly group 1", types.OriginOkta),
+		},
+	}
+	resourceIDs := []types.ResourceID{
+		newResourceID(clusterName, types.KindNode, "node1"),
+		newResourceID(clusterName, types.KindApp, "app1"),
+		newResourceID(clusterName, types.KindApp, "app2"),
+		newResourceID(clusterName, types.KindApp, "app3"),
+		newResourceID(clusterName, types.KindUserGroup, "group1"),
+	}
+
+	ctx := context.Background()
+
+	details, err := GetResourceDetails(ctx, clusterName, presence, resourceIDs)
+	require.NoError(t, err)
+
+	// Check the resource details to see if friendly names properly propagated.
+
+	// Node should be named for its hostname.
+	require.Equal(t, "hostname 1", details[types.ResourceIDToString(resourceIDs[0])].FriendlyName)
+
+	// app1 and app2 are expected to be empty because they're not Okta sourced resources.
+	require.Empty(t, details[types.ResourceIDToString(resourceIDs[1])].FriendlyName)
+
+	require.Empty(t, details[types.ResourceIDToString(resourceIDs[2])].FriendlyName)
+
+	// This Okta sourced app should have a friendly name.
+	require.Equal(t, "friendly app 3", details[types.ResourceIDToString(resourceIDs[3])].FriendlyName)
+
+	// This Okta sourced user group should have a friendly name.
+	require.Equal(t, "friendly group 1", details[types.ResourceIDToString(resourceIDs[4])].FriendlyName)
+}

api/types/resource.go

@@ -654,3 +654,18 @@ func ValidateResourceName(validationRegex *regexp.Regexp, name string) error {
 		name, validationRegex.String(),
 	)
 }
+
+// FriendlyName will return the friendly name for a resource if it has one. Otherwise, it
+// will return an empty string.
+func FriendlyName(resource ResourceWithLabels) string {
+	// Right now, only resources sourced from Okta and nodes have friendly names.
+	if resource.Origin() == OriginOkta {
+		return resource.GetMetadata().Description
+	}
+
+	if hn, ok := resource.(interface{ GetHostname() string }); ok {
+		return hn.GetHostname()
+	}
+
+	return ""
+}

api/types/resource_test.go

@@ -501,3 +501,59 @@ func TestValidLabelKey(t *testing.T) {
 		require.Equal(t, tc.valid, isValid)
 	}
 }
+
+func TestFriendlyName(t *testing.T) {
+	appNoFriendly, err := NewAppV3(Metadata{
+		Name: "no friendly",
+	}, AppSpecV3{
+		URI: "https://some-uri.com",
+	},
+	)
+	require.NoError(t, err)
+
+	appFriendly, err := NewAppV3(Metadata{
+		Name:        "no friendly",
+		Description: "friendly name",
+		Labels: map[string]string{
+			OriginLabel: OriginOkta,
+		},
+	}, AppSpecV3{
+		URI: "https://some-uri.com",
+	},
+	)
+	require.NoError(t, err)
+
+	node, err := NewServer("node", KindNode, ServerSpecV2{
+		Hostname: "friendly hostname",
+	})
+	require.NoError(t, err)
+
+	tests := []struct {
+		name     string
+		resource ResourceWithLabels
+		expected string
+	}{
+		{
+			name:     "no friendly name",
+			resource: appNoFriendly,
+			expected: "",
+		},
+		{
+			name:     "friendly app name",
+			resource: appFriendly,
+			expected: "friendly name",
+		},
+		{
+			name:     "friendly node name",
+			resource: node,
+			expected: "friendly hostname",
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			require.Equal(t, test.expected, FriendlyName(test.resource))
+		})
+	}
+}

integration/helpers/helpers.go

@@ -35,6 +35,7 @@ import (
 	"golang.org/x/crypto/ssh/agent"
 
 	"github.com/gravitational/teleport/api/breaker"
+	apiclient "github.com/gravitational/teleport/api/client"
 	"github.com/gravitational/teleport/api/client/proto"
 	"github.com/gravitational/teleport/api/constants"
 	apidefaults "github.com/gravitational/teleport/api/defaults"
@@ -473,7 +474,7 @@ func MustCreateListener(t *testing.T) net.Listener {
 	return listener
 }
 
-func FindNodeWithLabel(t *testing.T, ctx context.Context, cl services.ResourceLister, key, value string) func() bool {
+func FindNodeWithLabel(t *testing.T, ctx context.Context, cl apiclient.ListResourcesClient, key, value string) func() bool {
 	t.Helper()
 	return func() bool {
 		servers, err := cl.ListResources(ctx, proto.ListResourcesRequest{

integrations/access/common/app.go

@@ -18,10 +18,12 @@ package common
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"github.com/gravitational/trace"
 
+	"github.com/gravitational/teleport/api/accessrequest"
 	"github.com/gravitational/teleport/api/client/proto"
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/integrations/access/common/teleport"
@@ -242,14 +244,21 @@ func (a *BaseApp) onPendingRequest(ctx context.Context, req types.AccessRequest)
 	log := logger.Get(ctx)
 
 	reqID := req.GetName()
+
+	resourceNames, err := a.getResourceNames(ctx, req)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+
 	reqData := pd.AccessRequestData{
 		User:              req.GetUser(),
 		Roles:             req.GetRoles(),
 		RequestReason:     req.GetRequestReason(),
 		SystemAnnotations: req.GetSystemAnnotations(),
+		Resources:         resourceNames,
 	}
 
-	_, err := a.pluginData.Create(ctx, reqID, GenericPluginData{AccessRequestData: reqData})
+	_, err = a.pluginData.Create(ctx, reqID, GenericPluginData{AccessRequestData: reqData})
 	switch {
 	case err == nil:
 		// This is a new access-request, we have to broadcast it first.
@@ -479,3 +488,24 @@ func (a *BaseApp) updateMessages(ctx context.Context, reqID string, tag pd.Resol
 
 	return nil
 }
+
+func (a *BaseApp) getResourceNames(ctx context.Context, req types.AccessRequest) ([]string, error) {
+	resourceNames := make([]string, 0, len(req.GetRequestedResourceIDs()))
+	resourcesByCluster := accessrequest.GetResourceIDsByCluster(req)
+
+	for cluster, resources := range resourcesByCluster {
+		resourceDetails, err := accessrequest.GetResourceDetails(ctx, cluster, a.apiClient, resources)
+		if err != nil {
+			return nil, trace.Wrap(err)
+		}
+
+		for _, resource := range resources {
+			resourceName := types.ResourceIDToString(resource)
+			if details, ok := resourceDetails[resourceName]; ok && details.FriendlyName != "" {
+				resourceName = fmt.Sprintf("%s/%s", resource.Kind, details.FriendlyName)
+			}
+			resourceNames = append(resourceNames, resourceName)
+		}
+	}
+	return resourceNames, nil
+}

integrations/access/common/message.go

@@ -78,9 +78,12 @@ func MsgFields(reqID string, reqData pd.AccessRequestData, clusterName string, w
 	if len(reqData.User) > 0 {
 		msgFieldToBuilder(&builder, "User", reqData.User)
 	}
-	if reqData.Roles != nil {
+	if len(reqData.Roles) > 0 {
 		msgFieldToBuilder(&builder, "Role(s)", strings.Join(reqData.Roles, ","))
 	}
+	if len(reqData.Resources) > 0 {
+		msgFieldToBuilder(&builder, "Resource(s)", strings.Join(reqData.Resources, ","))
+	}
 	if reqData.RequestReason != "" {
 		msgFieldToBuilder(&builder, "Reason", lib.MarkdownEscape(reqData.RequestReason, requestReasonLimit))
 	}

integrations/access/common/plugindata_test.go

@@ -26,6 +26,7 @@ var samplePluginData = GenericPluginData{
 	AccessRequestData: plugindata.AccessRequestData{
 		User:             "user-foo",
 		Roles:            []string{"role-foo", "role-bar"},
+		Resources:        []string{"cluster-a/node/foo", "cluster-a/node/bar"},
 		RequestReason:    "foo reason",
 		ReviewsCount:     3,
 		ResolutionTag:    plugindata.ResolvedApproved,
@@ -40,9 +41,10 @@ var samplePluginData = GenericPluginData{
 func TestEncodePluginData(t *testing.T) {
 	dataMap, err := EncodePluginData(samplePluginData)
 	assert.NoError(t, err)
-	assert.Len(t, dataMap, 7)
+	assert.Len(t, dataMap, 8)
 	assert.Equal(t, "user-foo", dataMap["user"])
 	assert.Equal(t, "role-foo,role-bar", dataMap["roles"])
+	assert.Equal(t, `["cluster-a/node/foo","cluster-a/node/bar"]`, dataMap["resources"])
 	assert.Equal(t, "foo reason", dataMap["request_reason"])
 	assert.Equal(t, "3", dataMap["reviews_count"])
 	assert.Equal(t, "APPROVED", dataMap["resolution"])
@@ -54,6 +56,7 @@ func TestDecodePluginData(t *testing.T) {
 	pluginData, err := DecodePluginData(map[string]string{
 		"user":           "user-foo",
 		"roles":          "role-foo,role-bar",
+		"resources":      `["cluster-a/node/foo","cluster-a/node/bar"]`,
 		"request_reason": "foo reason",
 		"reviews_count":  "3",
 		"resolution":     "APPROVED",
@@ -67,7 +70,7 @@ func TestDecodePluginData(t *testing.T) {
 func TestEncodeEmptyPluginData(t *testing.T) {
 	dataMap, err := EncodePluginData(GenericPluginData{})
 	assert.NoError(t, err)
-	assert.Len(t, dataMap, 7)
+	assert.Len(t, dataMap, 8)
 	for key, value := range dataMap {
 		assert.Emptyf(t, value, "value at key %q must be empty", key)
 	}

integrations/access/common/teleport/client.go

@@ -32,4 +32,5 @@ type Client interface {
 	GetAccessRequests(ctx context.Context, filter types.AccessRequestFilter) ([]types.AccessRequest, error)
 	SubmitAccessReview(ctx context.Context, params types.AccessReviewSubmission) (types.AccessRequest, error)
 	SetAccessRequestState(ctx context.Context, params types.AccessRequestUpdate) error
+	ListResources(ctx context.Context, req proto.ListResourcesRequest) (*types.ListResourcesResponse, error)
 }

integrations/lib/plugindata/access_request.go

@@ -42,6 +42,7 @@ type AccessRequestData struct {
 	ResolutionTag     ResolutionTag
 	ResolutionReason  string
 	SystemAnnotations map[string][]string
+	Resources         []string
 }
 
 // DecodeAccessRequestData deserializes a string map to PluginData struct.
@@ -57,8 +58,16 @@ func DecodeAccessRequestData(dataMap map[string]string) (data AccessRequestData,
 	data.ResolutionTag = ResolutionTag(dataMap["resolution"])
 	data.ResolutionReason = dataMap["resolve_reason"]
 
-	if _, ok := dataMap["system_annotations"]; ok {
-		err = json.Unmarshal([]byte(dataMap["system_annotations"]), &data.SystemAnnotations)
+	if str, ok := dataMap["resources"]; ok {
+		err = json.Unmarshal([]byte(str), &data.Resources)
+		if err != nil {
+			err = trace.Wrap(err)
+			return
+		}
+	}
+
+	if str, ok := dataMap["system_annotations"]; ok {
+		err = json.Unmarshal([]byte(str), &data.SystemAnnotations)
 		if err != nil {
 			err = trace.Wrap(err)
 			return
@@ -76,8 +85,17 @@ func EncodeAccessRequestData(data AccessRequestData) (map[string]string, error)
 
 	result["user"] = data.User
 	result["roles"] = strings.Join(data.Roles, ",")
+	result["resources"] = strings.Join(data.Resources, ",")
 	result["request_reason"] = data.RequestReason
 
+	if len(data.Resources) != 0 {
+		resources, err := json.Marshal(data.Resources)
+		if err != nil {
+			return nil, trace.Wrap(err)
+		}
+		result["resources"] = string(resources)
+	}
+
 	var reviewsCountStr string
 	if data.ReviewsCount > 0 {
 		reviewsCountStr = fmt.Sprintf("%d", data.ReviewsCount)

integrations/lib/plugindata/access_request_test.go

@@ -23,6 +23,7 @@ import (
 var sampleAccessRequestData = AccessRequestData{
 	User:             "user-foo",
 	Roles:            []string{"role-foo", "role-bar"},
+	Resources:        []string{"cluster/node/foo", "cluster/node/bar"},
 	RequestReason:    "foo reason",
 	ReviewsCount:     3,
 	ResolutionTag:    ResolvedApproved,
@@ -32,9 +33,10 @@ var sampleAccessRequestData = AccessRequestData{
 func TestEncodeAccessRequestData(t *testing.T) {
 	dataMap, err := EncodeAccessRequestData(sampleAccessRequestData)
 	assert.Nil(t, err)
-	assert.Len(t, dataMap, 6)
+	assert.Len(t, dataMap, 7)
 	assert.Equal(t, "user-foo", dataMap["user"])
 	assert.Equal(t, "role-foo,role-bar", dataMap["roles"])
+	assert.Equal(t, `["cluster/node/foo","cluster/node/bar"]`, dataMap["resources"])
 	assert.Equal(t, "foo reason", dataMap["request_reason"])
 	assert.Equal(t, "3", dataMap["reviews_count"])
 	assert.Equal(t, "APPROVED", dataMap["resolution"])
@@ -45,6 +47,7 @@ func TestDecodeAccessRequestData(t *testing.T) {
 	pluginData, err := DecodeAccessRequestData(map[string]string{
 		"user":           "user-foo",
 		"roles":          "role-foo,role-bar",
+		"resources":      `["cluster/node/foo", "cluster/node/bar"]`,
 		"request_reason": "foo reason",
 		"reviews_count":  "3",
 		"resolution":     "APPROVED",
@@ -57,7 +60,7 @@ func TestDecodeAccessRequestData(t *testing.T) {
 func TestEncodeEmptyAccessRequestData(t *testing.T) {
 	dataMap, err := EncodeAccessRequestData(AccessRequestData{})
 	assert.Nil(t, err)
-	assert.Len(t, dataMap, 6)
+	assert.Len(t, dataMap, 7)
 	for key, value := range dataMap {
 		assert.Emptyf(t, value, "value at key %q must be empty", key)
 	}

lib/services/access_request.go

@@ -18,7 +18,6 @@ package services
 
 import (
 	"context"
-	"fmt"
 	"sort"
 	"strings"
 	"time"
@@ -30,7 +29,8 @@ import (
 	"github.com/vulcand/predicate"
 	"golang.org/x/exp/slices"
 
-	"github.com/gravitational/teleport/api/client/proto"
+	"github.com/gravitational/teleport/api/accessrequest"
+	"github.com/gravitational/teleport/api/client"
 	apidefaults "github.com/gravitational/teleport/api/defaults"
 	"github.com/gravitational/teleport/api/types"
 	apiutils "github.com/gravitational/teleport/api/utils"
@@ -719,17 +719,12 @@ func GetTraitMappings(cms []types.ClaimMapping) types.TraitMappingSet {
 	return types.TraitMappingSet(tm)
 }
 
-// ResourceLister is an interface which can list resources.
-type ResourceLister interface {
-	ListResources(ctx context.Context, req proto.ListResourcesRequest) (*types.ListResourcesResponse, error)
-}
-
 // RequestValidatorGetter is the interface required by the request validation
 // functions used to get necessary resources.
 type RequestValidatorGetter interface {
 	UserGetter
 	RoleGetter
-	ResourceLister
+	client.ListResourcesClient
 	GetRoles(ctx context.Context) ([]types.Role, error)
 	GetClusterName(opts ...MarshalOption) (types.ClusterName, error)
 }
@@ -1815,138 +1810,14 @@ func (m *RequestValidator) roleAllowsResource(
 	return true, nil
 }
 
-type ListResourcesRequestOption func(*proto.ListResourcesRequest)
-
-func GetResourceDetails(ctx context.Context, clusterName string, lister ResourceLister, ids []types.ResourceID) (map[string]types.ResourceDetails, error) {
-	var resourceIDs []types.ResourceID
-	for _, resourceID := range ids {
-		// We're interested in hostname or friendly name details. These apply to
-		// nodes, app servers, and user groups.
-		switch resourceID.Kind {
-		case types.KindNode, types.KindApp, types.KindUserGroup:
-			resourceIDs = append(resourceIDs, resourceID)
-		}
-	}
-
-	withExtraRoles := func(req *proto.ListResourcesRequest) {
-		req.UseSearchAsRoles = true
-		req.UsePreviewAsRoles = true
-	}
-
-	resources, err := GetResourcesByResourceIDs(ctx, lister, resourceIDs, withExtraRoles)
-	if err != nil {
-		return nil, trace.Wrap(err)
-	}
-
-	result := make(map[string]types.ResourceDetails)
-	for _, resource := range resources {
-		friendlyName := FriendlyName(resource)
-
-		// No friendly name was found, so skip to the next resource.
-		if friendlyName == "" {
-			continue
-		}
-
-		id := types.ResourceID{
-			ClusterName: clusterName,
-			Kind:        resource.GetKind(),
-			Name:        resource.GetName(),
-		}
-		result[types.ResourceIDToString(id)] = types.ResourceDetails{
-			FriendlyName: friendlyName,
-		}
-	}
-
-	return result, nil
+// TODO(atburke): Remove this once teleport.e reference is switched over
+func GetResourceDetails(ctx context.Context, clusterName string, lister client.ListResourcesClient, ids []types.ResourceID) (map[string]types.ResourceDetails, error) {
+	return accessrequest.GetResourceDetails(ctx, clusterName, lister, ids)
 }
 
-// GetResourceIDsByCluster will return resource IDs grouped by cluster.
+// TODO(atburke): Remove this once teleport.e reference is switched over
 func GetResourceIDsByCluster(r types.AccessRequest) map[string][]types.ResourceID {
-	resourceIDsByCluster := make(map[string][]types.ResourceID)
-	for _, resourceID := range r.GetRequestedResourceIDs() {
-		resourceIDsByCluster[resourceID.ClusterName] = append(resourceIDsByCluster[resourceID.ClusterName], resourceID)
-	}
-	return resourceIDsByCluster
-}
-
-func GetResourcesByResourceIDs(ctx context.Context, lister ResourceLister, resourceIDs []types.ResourceID, opts ...ListResourcesRequestOption) ([]types.ResourceWithLabels, error) {
-	resourceNamesByKind := make(map[string][]string)
-	for _, resourceID := range resourceIDs {
-		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)),
-		}
-		for _, opt := range opts {
-			opt(&req)
-		}
-		resp, err := lister.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)
-			}
-			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(`resource.metadata.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.KindKubeServer
-	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.KubeServer:
-		return types.ResourcesWithLabels{r.GetCluster()}, nil
-	case types.DatabaseServer:
-		return types.ResourcesWithLabels{r.GetDatabase()}, nil
-	case types.Server:
-		if hint == types.KindKubernetesCluster {
-			return nil, trace.BadParameter("expected kubernetes server, got server")
-		}
-	default:
-	}
-	return types.ResourcesWithLabels{resource}, nil
+	return accessrequest.GetResourceIDsByCluster(r)
 }
 
 // resourceMatcherToMatcherSlice returns the resourceMatcher in a RoleMatcher slice
@@ -1974,7 +1845,7 @@ func (m *RequestValidator) getUnderlyingResourcesByResourceIDs(ctx context.Conte
 		}
 	}
 	// load the underlying resources.
-	resources, err := GetResourcesByResourceIDs(ctx, m.getter, searchableResourcesIDs)
+	resources, err := accessrequest.GetResourcesByResourceIDs(ctx, m.getter, searchableResourcesIDs)
 	return resources, trace.Wrap(err)
 }
 

lib/services/access_request_test.go

@@ -1888,58 +1888,6 @@ func TestValidateAccessRequestClusterNames(t *testing.T) {
 	}
 }
 
-type mockResourceLister struct {
-	resources []types.ResourceWithLabels
-}
-
-func (m *mockResourceLister) ListResources(ctx context.Context, _ proto.ListResourcesRequest) (*types.ListResourcesResponse, error) {
-	return &types.ListResourcesResponse{
-		Resources: m.resources,
-	}, nil
-}
-
-func TestGetResourceDetails(t *testing.T) {
-	clusterName := "cluster"
-
-	presence := &mockResourceLister{
-		resources: []types.ResourceWithLabels{
-			newNode(t, "node1", "hostname 1"),
-			newApp(t, "app1", "friendly app 1", types.OriginDynamic),
-			newApp(t, "app2", "friendly app 2", types.OriginDynamic),
-			newApp(t, "app3", "friendly app 3", types.OriginOkta),
-			newUserGroup(t, "group1", "friendly group 1", types.OriginOkta),
-		},
-	}
-	resourceIDs := []types.ResourceID{
-		newResourceID(clusterName, types.KindNode, "node1"),
-		newResourceID(clusterName, types.KindApp, "app1"),
-		newResourceID(clusterName, types.KindApp, "app2"),
-		newResourceID(clusterName, types.KindApp, "app3"),
-		newResourceID(clusterName, types.KindUserGroup, "group1"),
-	}
-
-	ctx := context.Background()
-
-	details, err := GetResourceDetails(ctx, clusterName, presence, resourceIDs)
-	require.NoError(t, err)
-
-	// Check the resource details to see if friendly names properly propagated.
-
-	// Node should be named for its hostname.
-	require.Equal(t, "hostname 1", details[types.ResourceIDToString(resourceIDs[0])].FriendlyName)
-
-	// app1 and app2 are expected to be empty because they're not Okta sourced resources.
-	require.Empty(t, details[types.ResourceIDToString(resourceIDs[1])].FriendlyName)
-
-	require.Empty(t, details[types.ResourceIDToString(resourceIDs[2])].FriendlyName)
-
-	// This Okta sourced app should have a friendly name.
-	require.Equal(t, "friendly app 3", details[types.ResourceIDToString(resourceIDs[3])].FriendlyName)
-
-	// This Okta sourced user group should have a friendly name.
-	require.Equal(t, "friendly group 1", details[types.ResourceIDToString(resourceIDs[4])].FriendlyName)
-}
-
 func TestMaxDuration(t *testing.T) {
 	// describes a collection of roles and their conditions
 	roleDesc := roleTestSet{
@@ -2175,48 +2123,3 @@ func getMockGetter(t *testing.T, roleDesc roleTestSet, userDesc map[string][]str
 	}
 	return g
 }
-
-func newNode(t *testing.T, name, hostname string) types.Server {
-	node, err := types.NewServer(name, types.KindNode,
-		types.ServerSpecV2{
-			Hostname: hostname,
-		})
-	require.NoError(t, err)
-	return node
-}
-
-func newApp(t *testing.T, name, description, origin string) types.Application {
-	app, err := types.NewAppV3(types.Metadata{
-		Name:        name,
-		Description: description,
-		Labels: map[string]string{
-			types.OriginLabel: origin,
-		},
-	},
-		types.AppSpecV3{
-			URI:        "https://some-addr.com",
-			PublicAddr: "https://some-addr.com",
-		})
-	require.NoError(t, err)
-	return app
-}
-
-func newUserGroup(t *testing.T, name, description, origin string) types.UserGroup {
-	userGroup, err := types.NewUserGroup(types.Metadata{
-		Name:        name,
-		Description: description,
-		Labels: map[string]string{
-			types.OriginLabel: origin,
-		},
-	}, types.UserGroupSpecV1{})
-	require.NoError(t, err)
-	return userGroup
-}
-
-func newResourceID(clusterName, kind, name string) types.ResourceID {
-	return types.ResourceID{
-		ClusterName: clusterName,
-		Kind:        kind,
-		Name:        name,
-	}
-}

lib/services/friendlyname.go

@@ -1,34 +0,0 @@
-/*
-Copyright 2023 Gravitational, Inc.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package services
-
-import "github.com/gravitational/teleport/api/types"
-
-// FriendlyName will return the friendly name for a resource if it has one. Otherwise, it
-// will return an empty string.
-func FriendlyName(resource types.ResourceWithLabels) string {
-	// Right now, only resources sourced from Okta and nodes have friendly names.
-	if resource.Origin() == types.OriginOkta {
-		return resource.GetMetadata().Description
-	}
-
-	if hn, ok := resource.(interface{ GetHostname() string }); ok {
-		return hn.GetHostname()
-	}
-
-	return ""
-}

lib/services/friendlyname_test.go

@@ -1,81 +0,0 @@
-/*
-Copyright 2023 Gravitational, Inc.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package services
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/require"
-
-	"github.com/gravitational/teleport/api/types"
-)
-
-func TestFriendlyName(t *testing.T) {
-	appNoFriendly, err := types.NewAppV3(types.Metadata{
-		Name: "no friendly",
-	}, types.AppSpecV3{
-		URI: "https://some-uri.com",
-	},
-	)
-	require.NoError(t, err)
-
-	appFriendly, err := types.NewAppV3(types.Metadata{
-		Name:        "no friendly",
-		Description: "friendly name",
-		Labels: map[string]string{
-			types.OriginLabel: types.OriginOkta,
-		},
-	}, types.AppSpecV3{
-		URI: "https://some-uri.com",
-	},
-	)
-	require.NoError(t, err)
-
-	node, err := types.NewServer("node", types.KindNode, types.ServerSpecV2{
-		Hostname: "friendly hostname",
-	})
-	require.NoError(t, err)
-
-	tests := []struct {
-		name     string
-		resource types.ResourceWithLabels
-		expected string
-	}{
-		{
-			name:     "no friendly name",
-			resource: appNoFriendly,
-			expected: "",
-		},
-		{
-			name:     "friendly app name",
-			resource: appFriendly,
-			expected: "friendly name",
-		},
-		{
-			name:     "friendly node name",
-			resource: node,
-			expected: "friendly hostname",
-		},
-	}
-
-	for _, test := range tests {
-		test := test
-		t.Run(test.name, func(t *testing.T) {
-			require.Equal(t, test.expected, FriendlyName(test.resource))
-		})
-	}
-}

lib/teleterm/clusters/cluster_access_requests.go

@@ -21,6 +21,7 @@ import (
 	"github.com/gravitational/trace"
 	"golang.org/x/exp/slices"
 
+	"github.com/gravitational/teleport/api/accessrequest"
 	"github.com/gravitational/teleport/api/types"
 	api "github.com/gravitational/teleport/gen/proto/go/teleport/lib/teleterm/v1"
 	"github.com/gravitational/teleport/lib/auth"
@@ -272,11 +273,11 @@ func (c *Cluster) AssumeRole(ctx context.Context, req *api.AssumeRoleRequest) er
 }
 
 func getResourceDetails(ctx context.Context, req types.AccessRequest, clt auth.ClientI) (map[string]ResourceDetails, error) {
-	resourceIDsByCluster := services.GetResourceIDsByCluster(req)
+	resourceIDsByCluster := accessrequest.GetResourceIDsByCluster(req)
 
 	resourceDetails := make(map[string]ResourceDetails)
 	for clusterName, resourceIDs := range resourceIDsByCluster {
-		details, err := services.GetResourceDetails(ctx, clusterName, clt, resourceIDs)
+		details, err := accessrequest.GetResourceDetails(ctx, clusterName, clt, resourceIDs)
 		if err != nil {
 			return nil, trace.Wrap(err)
 		}

lib/web/ui/app.go

@@ -23,7 +23,6 @@ import (
 	"github.com/sirupsen/logrus"
 
 	"github.com/gravitational/teleport/api/types"
-	"github.com/gravitational/teleport/lib/services"
 	"github.com/gravitational/teleport/lib/tlsca"
 	"github.com/gravitational/teleport/lib/utils/aws"
 )
@@ -123,7 +122,7 @@ func MakeApp(app types.Application, c MakeAppsConfig) App {
 		ClusterID:    c.AppClusterName,
 		FQDN:         fqdn,
 		AWSConsole:   app.IsAWSConsole(),
-		FriendlyName: services.FriendlyName(app),
+		FriendlyName: types.FriendlyName(app),
 		UserGroups:   userGroupAndDescriptions,
 		SAMLApp:      false,
 	}
@@ -146,7 +145,7 @@ func MakeSAMLApp(app types.SAMLIdPServiceProvider, c MakeAppsConfig) App {
 		PublicAddr:   "",
 		Labels:       labels,
 		ClusterID:    c.AppClusterName,
-		FriendlyName: services.FriendlyName(app),
+		FriendlyName: types.FriendlyName(app),
 		SAMLApp:      true,
 	}
 
@@ -182,7 +181,7 @@ func MakeApps(c MakeAppsConfig) []App {
 				ClusterID:    c.AppClusterName,
 				FQDN:         fqdn,
 				AWSConsole:   app.IsAWSConsole(),
-				FriendlyName: services.FriendlyName(app),
+				FriendlyName: types.FriendlyName(app),
 				UserGroups:   userGroupAndDescriptions,
 				SAMLApp:      false,
 			}
@@ -202,7 +201,7 @@ func MakeApps(c MakeAppsConfig) []App {
 				PublicAddr:   appOrSP.GetPublicAddr(),
 				Labels:       labels,
 				ClusterID:    c.AppClusterName,
-				FriendlyName: services.FriendlyName(appOrSP),
+				FriendlyName: types.FriendlyName(appOrSP),
 				SAMLApp:      true,
 			}
 

lib/web/ui/user_groups.go

@@ -18,7 +18,6 @@ package ui
 
 import (
 	"github.com/gravitational/teleport/api/types"
-	"github.com/gravitational/teleport/lib/services"
 )
 
 // UserGroup describes a user group.
@@ -54,7 +53,7 @@ func MakeUserGroups(userGroups []types.UserGroup, userGroupsToApps map[string]ty
 		for i, app := range apps {
 			appsAndFriendlyNames[i] = ApplicationAndFriendlyName{
 				Name:         app.GetName(),
-				FriendlyName: services.FriendlyName(app),
+				FriendlyName: types.FriendlyName(app),
 			}
 		}
 
@@ -62,7 +61,7 @@ func MakeUserGroups(userGroups []types.UserGroup, userGroupsToApps map[string]ty
 			Name:         userGroup.GetName(),
 			Description:  userGroup.GetMetadata().Description,
 			Labels:       uiLabels,
-			FriendlyName: services.FriendlyName(userGroup),
+			FriendlyName: types.FriendlyName(userGroup),
 			Applications: appsAndFriendlyNames,
 		})
 	}

tool/tsh/common/access_request.go

@@ -28,6 +28,7 @@ import (
 	"golang.org/x/exp/slices"
 
 	"github.com/gravitational/teleport"
+	"github.com/gravitational/teleport/api/accessrequest"
 	"github.com/gravitational/teleport/api/client"
 	"github.com/gravitational/teleport/api/client/proto"
 	kubeproto "github.com/gravitational/teleport/api/gen/proto/go/teleport/kube/v1"
@@ -434,24 +435,16 @@ func onRequestSearch(cf *CLIConf) error {
 		authClient := proxyClient.CurrentCluster()
 
 		req := proto.ListResourcesRequest{
-			ResourceType:        services.MapResourceKindToListResourcesType(cf.ResourceKind),
 			Labels:              tc.Labels,
 			PredicateExpression: cf.PredicateExpression,
 			SearchKeywords:      tc.SearchKeywords,
 			UseSearchAsRoles:    true,
 		}
 
-		results, err := client.GetResourcesWithFilters(cf.Context, authClient, req)
+		resources, err = accessrequest.GetResourcesByKind(cf.Context, authClient, req, cf.ResourceKind)
 		if err != nil {
 			return trace.Wrap(err)
 		}
-		for _, result := range results {
-			leafResources, err := services.MapListResourcesResultToLeafResource(result, cf.ResourceKind)
-			if err != nil {
-				return trace.Wrap(err)
-			}
-			resources = append(resources, leafResources...)
-		}
 		tableColumns = []string{"Name", "Hostname", "Labels", "Resource ID"}
 	}